On Thu, Oct 12, 2023 at 8:07 AM Marlon Rodriguez Garcia <
[email protected]> wrote:

> On Thu, Oct 12, 2023 at 10:47 AM, Tim Orling wrote:
>
> Thank you for working on Toaster. It is nice to see some new names and new
> efforts.
>
> Please split this into three patches, each with appropriate comments. It
> is hard to review such a large patch and each of the following is better
> addressed separately. We also need the git history to remind our future
> selves what each change was doing.
>
> (1) jquery update. Please add in the git log what the old version was and
> what the new version is and provide at least a link to the release notes
> for the changes. We need to be able to quickly see if there are any
> breaking changes that we need to be aware of or any bugs or security
> vulnerabilities that might have been fixed.
> (2) bootstrap update. Same as above
> (3) test changes: with the justification for adding the timing/sleep.
> Sleep delays are notoriously brittle and can depend on the loading of the
> machine. We may have to revisit these when the tests are running on the
> Yocto Project Auto Builder, as those systems are often under heavy load.
>
> Please resend to the mailing list. This is how the project communicates.
>
> On Thu, Oct 12, 2023 at 7:32 AM Marlon Rodriguez Garcia <
> [email protected]> wrote:
>
>> Hi Richard
>> I create a new version of the patch in here
>> <https://lists.yoctoproject.org/g/toaster/topic/patch_3_3_toaster_update/101919066?p=,,,20,0,0,0::recentpostdate/sticky,,,20,2,0,101919066,previd%3D1697120960078919554,nextid%3D1668382209711438099&previd=1697120960078919554&nextid=1668382209711438099>
>>
>> Please , let me know if this works ?
>>
>> Thanks
>>
>>
>>
>>  Hi,
> I understand is a little complicate to review patches with huge files,
> however in this case is necessary to update both JQuery and bootstrap in
> one instance because the update of JQuery creates incompatibility errors
> that are solve in bootstrap 3.3.7.
> These 2 changes must be done together to avoid introducing new issues.
>
> The two patches will be simultaneously merged, but they are two separate
updates. I am fully aware that they need to go hand in hand. We often have
a patch series where things build on prior patches in the same series.


> Related to the test, we are working to fix the entire test suite for
> toaster, if this is truly and issue i will split the patches.
>
>
We need to get into a pattern of patches that are easier to review and also
be able to in the future understand what happened when. I realize it is
more work to split them up, but it will get you into a better habit of
commits in the future. For each patch you should ask "is this one idea or
several". In this case this is 3 very separate ideas. Every one of us went
through this learning process as we started contributing.

Its possible to remove the previous patch i added from the mailing list
> becasue at this point is creating noise and i dont know if i can do it ?
>
>
No, once emails are out there, they are part of the historical record.
However,  in the future I will double check for a new patch and respond in
the new thread.

Thanks for the feedback and will improve the creation of changes in the
> following patches
>
>
> 
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#5853): https://lists.yoctoproject.org/g/toaster/message/5853
Mute This Topic: https://lists.yoctoproject.org/mt/101904715/21656
Group Owner: [email protected]
Unsubscribe: https://lists.yoctoproject.org/g/toaster/unsub 
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to