Re: Please don't abuse "No bug" in commit messages
On 02/06/2017 08:19 AM, Mike Hoye wrote: On 2017-02-03 2:01 PM, Steve Fink wrote: Typo/whitespace - No bug is fine, but if it is associated with a recent landing of some bug, then you should use that bug number anyway. Makes uplifts cleaner. Whitespace errors seem like a wierd thing for an engineer to be working on?. Code cleanup bugs are how a lot of people get started, and could also have automatic formatting. Employees pushing whitespace around seems like a distant third-best. While I also don't want people to suffer from unnecessary process overhead, I also want small-change bugs to be discoverable by people trying to start contributing. I'm not sure where that line is, but changes without bugs means nobody else could have helped make those changes. The scenario I'm thinking of is that I land something, and notice (or somebody points out) that I added whitespace at the end of a line somewhere. Landing an immediate fix seems like the lowest overhead way of dealing with it, other than ignoring it entirely. But ignoring it would cause a small amount of friction for other people, so I'd rather clean up after myself if I notice. A related situation is when starting to work on something, you find it has trailing whitespace scattered around. I like to have a separate whitespace fixup commit, to make the following changes more clear. That's the case where I wouldn't look up the bug where the whitespace was landed. (First, because it's a bit of nuisance to look up, and second, associating it with some old bug is going to cause more harm than good, especially if things have modified stuff in between.) I'm not really sure whether it's better to label such a changeset with "No bug" or with the new bug from the following changeset. I guess the latter would be better for uplifts. (Though I'll often squash things that I may want to uplift and attach that as an upliftable patch so I don't have to request approval on multiple patches and it's just easier to deal with one patch at a time.) Otherwise, yeah, I'm not going to go around looking for whitespace problems. I only deal with whitespace if it gets in my way, or if I recently introduced it. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Please don't abuse "No bug" in commit messages
On 2017-02-03 2:01 PM, Steve Fink wrote: Typo/whitespace - No bug is fine, but if it is associated with a recent landing of some bug, then you should use that bug number anyway. Makes uplifts cleaner. Whitespace errors seem like a wierd thing for an engineer to be working on?. Code cleanup bugs are how a lot of people get started, and could also have automatic formatting. Employees pushing whitespace around seems like a distant third-best. While I also don't want people to suffer from unnecessary process overhead, I also want small-change bugs to be discoverable by people trying to start contributing. I'm not sure where that line is, but changes without bugs means nobody else could have helped make those changes. - mhoye ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Please don't abuse "No bug" in commit messages
Gijs Kruitbosch writes: >> What if it causes a regression and a blocking bug needs to be filed? > Then you file a bug and needinfo the person who landed the commit > (which one would generally do anyway, besides just marking it > blocking the regressor). I find the association of multiple regressions with their cause very helpful because one regression is often similar to another. Bugzilla provides the link from cause to effect, a link direction which is not in the changeset history. Where there is no bug for the cause, then these links are missing. I'm not convinced that a "Changeset X bug" filed after the fact would be easy to find from the commit message. > More generally, I concur with Greg that we should pivot to having > the repos be our source of truth about what csets are present on > which branches. I've seen cases recently where e.g. we land a > feature, then there are regressions, and some of them are > addressed in followup bugs, and then eventually we back everything > out of one of the trains because we can't fix *all* the > regressions in time. At that point, the original feature bug's > flags are updated ('disabled' on the branch with backouts), but > not those of the regression fix deps, and so if *those* have > regressions, people filing bugs make incorrect assumptions about > what versions are affected. Manually tracking branch fix state in > bugzilla alone is a losing battle. A system for tracking related changes and branch state that can have mistakes corrected is better than no system at all. (I'm not clear whether you think the information currently in the repos is sufficient.) For exmample, using changeset history would be a cumbersome way to determine whether the most recent change for one issue on a branch was a "fix" or a back-out. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Please don't abuse "No bug" in commit messages
Ideally we would be doing it in a way that we can audit things quickly in the case we think a bad actor has compromised someone's machine. While we can say what we want and how we want the process, we need to work with what we have until we have a better process. This could be between now and the end of the year. Ideally, and hopefully its not too far away, all pushes will be going through autoland and then we won't have situations like this. In the short term, when the sheriffs do merges I will be getting them to have a look at all "no bug" pushes. Anything that goes outside of https://developer.mozilla.org/docs/Mozilla/Developer_guide/C ommitting_Rules_and_Responsibilities will be followed up. David On 3 February 2017 at 19:01, Steve Finkwrote: > On 02/03/2017 09:29 AM, Gijs Kruitbosch wrote: > >> On 03/02/2017 15:11, Ryan VanderMeulen wrote: >> >>> A friendly reminder that per the MDN commit rules, the use of "No bug" >>> in the commit message is to be used sparingly - in general for minor things >>> like whitespace changes/comment fixes/etc where traceability isn't as >>> important. >>> https://developer.mozilla.org/docs/Mozilla/Developer_guide/C >>> ommitting_Rules_and_Responsibilities >>> >>> I've come across uses of "No bug" commits recently where entire upstream >>> imports of vendored libraries were done. This is bad for multiple reasons: >>> * If makes sheriffing difficult - if the patch is broken and needs >>> backing out, where should the comment go? When it gets merged to >>> mozilla-central, who gets notified? >>> >> As Greg said, the committer / pusher, via IRC and/or email. >> >>> * It makes tracking a pain - what happens if that patch ends up needing >>> uplift? >>> >> Generally, the person committing it will know whether it needs uplift, >> and would have filed a bug if it did - and would file one if it does after >> the fact. We can already not rely on bugs being marked fixed/verified on a >> trunk branch when searching bugzilla for uplift requests (cf. "disable >> feature Y on beta" bugs) and so I don't see how this is relevant. >> >>> What about when that patch causes conflicts with another patch needing >>> uplift? >>> >> That seems like it hardly ever happens in the example you gave (vendored >> libraries and other wholesale "update this dir to match external canonical >> version"), and if/when it does, the people who would be likely to be >> involved in such changes (effectively changes to vendored deps that aren't >> copied from the same canonical source!) are also likely to be aware of what >> merged when. >> >>> What if it causes a regression and a blocking bug needs to be filed? >>> >> Then you file a bug and needinfo the person who landed the commit (which >> one would generally do anyway, besides just marking it blocking the >> regressor). >> >> >> If there's an overwhelming majority of people who think using "no bug" >> for landing 'sync m-c with repo X' commits is bad, we can make a more >> explicit change in the rules here. But in terms of reducing development >> friction, if we think bugs are necessary at this point, I would prefer >> doing something like what Greg suggested, ie auto-file bugs for commits >> that get pushed that don't have a bug associated with them. >> >> >> More generally, I concur with Greg that we should pivot to having the >> repos be our source of truth about what csets are present on which >> branches. I've seen cases recently where e.g. we land a feature, then there >> are regressions, and some of them are addressed in followup bugs, and then >> eventually we back everything out of one of the trains because we can't fix >> *all* the regressions in time. At that point, the original feature bug's >> flags are updated ('disabled' on the branch with backouts), but not those >> of the regression fix deps, and so if *those* have regressions, people >> filing bugs make incorrect assumptions about what versions are affected. >> Manually tracking branch fix state in bugzilla alone is a losing battle. >> > > For the immediate term, I must respectfully disagree. Sheriffs are the > people most involved with and concerned with the changeset management > procedures, so if the sheriffs (and Ryan "I'm not a sheriff!" VM) are > claiming that No Bug landings are being overused and causing issues, I'm > inclined to adjust current practices first and hold off on rethinking our > development process until the immediate pain is resolved. The fact is that > our *current* changeset management *is* very bug-centric. I think gps and > others have made a good argument for why another process may be superior > (and I personally do not regard our current process as the pinnacle of > efficiency), and I'm all for coming up with something better. But I really > don't want some long drawn-out halfway state where the people who think the > process should be A are doing it that way, the people who think it should > be B are changing piecemeal to get
Re: Please don't abuse "No bug" in commit messages
Thanks for checking the developer guidelines and making sure they are consistent. I'd like to hear a little more about: branch merges - … I personally wish that especially significant merges *did* have bugs to make it a little easier to track problems back to a branch landing and have a place for discussion of the reasons and risks. There's a discussion I've been in with release mangement about what we can be doing to improve these merges and how we are tracking them and the regressions they may cause. -- Emma On Fri, Feb 3, 2017 at 11:01 AM, Steve Finkwrote: > On 02/03/2017 09:29 AM, Gijs Kruitbosch wrote: > >> On 03/02/2017 15:11, Ryan VanderMeulen wrote: >> >>> A friendly reminder that per the MDN commit rules, the use of "No bug" >>> in the commit message is to be used sparingly - in general for minor things >>> like whitespace changes/comment fixes/etc where traceability isn't as >>> important. >>> https://developer.mozilla.org/docs/Mozilla/Developer_guide/C >>> ommitting_Rules_and_Responsibilities >>> >>> I've come across uses of "No bug" commits recently where entire upstream >>> imports of vendored libraries were done. This is bad for multiple reasons: >>> * If makes sheriffing difficult - if the patch is broken and needs >>> backing out, where should the comment go? When it gets merged to >>> mozilla-central, who gets notified? >>> >> As Greg said, the committer / pusher, via IRC and/or email. >> >>> * It makes tracking a pain - what happens if that patch ends up needing >>> uplift? >>> >> Generally, the person committing it will know whether it needs uplift, >> and would have filed a bug if it did - and would file one if it does after >> the fact. We can already not rely on bugs being marked fixed/verified on a >> trunk branch when searching bugzilla for uplift requests (cf. "disable >> feature Y on beta" bugs) and so I don't see how this is relevant. >> >>> What about when that patch causes conflicts with another patch needing >>> uplift? >>> >> That seems like it hardly ever happens in the example you gave (vendored >> libraries and other wholesale "update this dir to match external canonical >> version"), and if/when it does, the people who would be likely to be >> involved in such changes (effectively changes to vendored deps that aren't >> copied from the same canonical source!) are also likely to be aware of what >> merged when. >> >>> What if it causes a regression and a blocking bug needs to be filed? >>> >> Then you file a bug and needinfo the person who landed the commit (which >> one would generally do anyway, besides just marking it blocking the >> regressor). >> >> >> If there's an overwhelming majority of people who think using "no bug" >> for landing 'sync m-c with repo X' commits is bad, we can make a more >> explicit change in the rules here. But in terms of reducing development >> friction, if we think bugs are necessary at this point, I would prefer >> doing something like what Greg suggested, ie auto-file bugs for commits >> that get pushed that don't have a bug associated with them. >> >> >> More generally, I concur with Greg that we should pivot to having the >> repos be our source of truth about what csets are present on which >> branches. I've seen cases recently where e.g. we land a feature, then there >> are regressions, and some of them are addressed in followup bugs, and then >> eventually we back everything out of one of the trains because we can't fix >> *all* the regressions in time. At that point, the original feature bug's >> flags are updated ('disabled' on the branch with backouts), but not those >> of the regression fix deps, and so if *those* have regressions, people >> filing bugs make incorrect assumptions about what versions are affected. >> Manually tracking branch fix state in bugzilla alone is a losing battle. >> > > For the immediate term, I must respectfully disagree. Sheriffs are the > people most involved with and concerned with the changeset management > procedures, so if the sheriffs (and Ryan "I'm not a sheriff!" VM) are > claiming that No Bug landings are being overused and causing issues, I'm > inclined to adjust current practices first and hold off on rethinking our > development process until the immediate pain is resolved. The fact is that > our *current* changeset management *is* very bug-centric. I think gps and > others have made a good argument for why another process may be superior > (and I personally do not regard our current process as the pinnacle of > efficiency), and I'm all for coming up with something better. But I really > don't want some long drawn-out halfway state where the people who think the > process should be A are doing it that way, the people who think it should > be B are changing piecemeal to get something as close to B as is currently > possible, and the people new to our community are completely baffled and > getting contradictory advice depending on who they talk to. > > Can we clarify
Re: Please don't abuse "No bug" in commit messages
On 02/03/2017 09:29 AM, Gijs Kruitbosch wrote: On 03/02/2017 15:11, Ryan VanderMeulen wrote: A friendly reminder that per the MDN commit rules, the use of "No bug" in the commit message is to be used sparingly - in general for minor things like whitespace changes/comment fixes/etc where traceability isn't as important. https://developer.mozilla.org/docs/Mozilla/Developer_guide/Committing_Rules_and_Responsibilities I've come across uses of "No bug" commits recently where entire upstream imports of vendored libraries were done. This is bad for multiple reasons: * If makes sheriffing difficult - if the patch is broken and needs backing out, where should the comment go? When it gets merged to mozilla-central, who gets notified? As Greg said, the committer / pusher, via IRC and/or email. * It makes tracking a pain - what happens if that patch ends up needing uplift? Generally, the person committing it will know whether it needs uplift, and would have filed a bug if it did - and would file one if it does after the fact. We can already not rely on bugs being marked fixed/verified on a trunk branch when searching bugzilla for uplift requests (cf. "disable feature Y on beta" bugs) and so I don't see how this is relevant. What about when that patch causes conflicts with another patch needing uplift? That seems like it hardly ever happens in the example you gave (vendored libraries and other wholesale "update this dir to match external canonical version"), and if/when it does, the people who would be likely to be involved in such changes (effectively changes to vendored deps that aren't copied from the same canonical source!) are also likely to be aware of what merged when. What if it causes a regression and a blocking bug needs to be filed? Then you file a bug and needinfo the person who landed the commit (which one would generally do anyway, besides just marking it blocking the regressor). If there's an overwhelming majority of people who think using "no bug" for landing 'sync m-c with repo X' commits is bad, we can make a more explicit change in the rules here. But in terms of reducing development friction, if we think bugs are necessary at this point, I would prefer doing something like what Greg suggested, ie auto-file bugs for commits that get pushed that don't have a bug associated with them. More generally, I concur with Greg that we should pivot to having the repos be our source of truth about what csets are present on which branches. I've seen cases recently where e.g. we land a feature, then there are regressions, and some of them are addressed in followup bugs, and then eventually we back everything out of one of the trains because we can't fix *all* the regressions in time. At that point, the original feature bug's flags are updated ('disabled' on the branch with backouts), but not those of the regression fix deps, and so if *those* have regressions, people filing bugs make incorrect assumptions about what versions are affected. Manually tracking branch fix state in bugzilla alone is a losing battle. For the immediate term, I must respectfully disagree. Sheriffs are the people most involved with and concerned with the changeset management procedures, so if the sheriffs (and Ryan "I'm not a sheriff!" VM) are claiming that No Bug landings are being overused and causing issues, I'm inclined to adjust current practices first and hold off on rethinking our development process until the immediate pain is resolved. The fact is that our *current* changeset management *is* very bug-centric. I think gps and others have made a good argument for why another process may be superior (and I personally do not regard our current process as the pinnacle of efficiency), and I'm all for coming up with something better. But I really don't want some long drawn-out halfway state where the people who think the process should be A are doing it that way, the people who think it should be B are changing piecemeal to get something as close to B as is currently possible, and the people new to our community are completely baffled and getting contradictory advice depending on who they talk to. Can we clarify with examples of good and bad usages of "No bug"? There will always be a fuzzy middle (unless we go all-out and say every landing must be associated with a bug). But I'm hearing some dispute over specific scenarios, so I think it would be helpful to clarify. I am not the one to decide such things, but my personal guess would be: branch merges - No bug is fine. (And IIUC, you don't need to say "No bug" for those because the hook accepts /merge/ or something like that.) I personally wish that especially significant merges *did* have bugs to make it a little easier to track problems back to a branch landing and have a place for discussion of the reasons and risks. Typo/whitespace - No bug is fine, but if it is associated with a recent landing of some
Re: Please don't abuse "No bug" in commit messages
On 03/02/2017 15:11, Ryan VanderMeulen wrote: A friendly reminder that per the MDN commit rules, the use of "No bug" in the commit message is to be used sparingly - in general for minor things like whitespace changes/comment fixes/etc where traceability isn't as important. https://developer.mozilla.org/docs/Mozilla/Developer_guide/Committing_Rules_and_Responsibilities I've come across uses of "No bug" commits recently where entire upstream imports of vendored libraries were done. This is bad for multiple reasons: * If makes sheriffing difficult - if the patch is broken and needs backing out, where should the comment go? When it gets merged to mozilla-central, who gets notified? As Greg said, the committer / pusher, via IRC and/or email. * It makes tracking a pain - what happens if that patch ends up needing uplift? Generally, the person committing it will know whether it needs uplift, and would have filed a bug if it did - and would file one if it does after the fact. We can already not rely on bugs being marked fixed/verified on a trunk branch when searching bugzilla for uplift requests (cf. "disable feature Y on beta" bugs) and so I don't see how this is relevant. What about when that patch causes conflicts with another patch needing uplift? That seems like it hardly ever happens in the example you gave (vendored libraries and other wholesale "update this dir to match external canonical version"), and if/when it does, the people who would be likely to be involved in such changes (effectively changes to vendored deps that aren't copied from the same canonical source!) are also likely to be aware of what merged when. What if it causes a regression and a blocking bug needs to be filed? Then you file a bug and needinfo the person who landed the commit (which one would generally do anyway, besides just marking it blocking the regressor). If there's an overwhelming majority of people who think using "no bug" for landing 'sync m-c with repo X' commits is bad, we can make a more explicit change in the rules here. But in terms of reducing development friction, if we think bugs are necessary at this point, I would prefer doing something like what Greg suggested, ie auto-file bugs for commits that get pushed that don't have a bug associated with them. More generally, I concur with Greg that we should pivot to having the repos be our source of truth about what csets are present on which branches. I've seen cases recently where e.g. we land a feature, then there are regressions, and some of them are addressed in followup bugs, and then eventually we back everything out of one of the trains because we can't fix *all* the regressions in time. At that point, the original feature bug's flags are updated ('disabled' on the branch with backouts), but not those of the regression fix deps, and so if *those* have regressions, people filing bugs make incorrect assumptions about what versions are affected. Manually tracking branch fix state in bugzilla alone is a losing battle. ~ Gijs Bugs are cheap. Please use them. Thanks, Ryan ___ firefox-dev mailing list firefox-...@mozilla.org https://mail.mozilla.org/listinfo/firefox-dev ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Please don't abuse "No bug" in commit messages
On Fri, Feb 3, 2017 at 7:11 AM, Ryan VanderMeulenwrote: > A friendly reminder that per the MDN commit rules, the use of "No bug" in > the commit message is to be used sparingly - in general for minor things > like whitespace changes/comment fixes/etc where traceability isn't as > important. > https://developer.mozilla.org/docs/Mozilla/Developer_guide/C > ommitting_Rules_and_Responsibilities > > I've come across uses of "No bug" commits recently where entire upstream > imports of vendored libraries were done. This is bad for multiple reasons: > * If makes sheriffing difficult - if the patch is broken and needs backing > out, where should the comment go? When it gets merged to mozilla-central, > who gets notified? > The commit author and/or who pushed the commit. We have email addresses in the commit message and the pushlog. We should use them. There is a https://github.com/glandium/pulsebot feature request waiting to be filed. > * It makes tracking a pain - what happens if that patch ends up needing > uplift? What about when that patch causes conflicts with another patch > needing uplift? What if it causes a regression and a blocking bug needs to > be filed? > Then you file a new bug "Changeset XX " As I argue at http://gregoryszorc.com/blog/2015/01/16/bugzilla-and-the- future-of-firefox-development/ we've conflated bugs with changesets/landings and I don't think this is optimal. At the end of the day, the changeset/commit is the thing that matters because a bug doesn't directly change the behavior of Firefox. Instead, bugs are convenient anchor points for {discussion, tracking, review, etc} that just so happen to exist most of the time because 10+ years ago we tightly coupled our code review mechanism into Bugzilla, which requires the existence of a bug. > > Bugs are cheap. Please use them. > They are. But there is still a cost to them. While I agree that things like imports of upstream code are an abuse of "no bug," I feel strongly that we shouldn't institute avoidable process overhead, especially for things like trivial whitespace or comment changes. If we insist on having a bug for every changeset, then I insist we teach the review/landing process to create bugs automatically when a bugless changeset is seen so people aren't burdened with the overhead of doing that. We have most of the technical pieces in place to support this, including bug 1328351 annotating every file in the tree with a bug component. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Please don't abuse "No bug" in commit messages
I have raised a bug[1] to block these types of commits in the future. This is an unnecessary risk that we are taking. I also think that we need to remove this as acceptable practice from the MDN page. David [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1336459 On 3 February 2017 at 15:11, Ryan VanderMeulenwrote: > A friendly reminder that per the MDN commit rules, the use of "No bug" in > the commit message is to be used sparingly - in general for minor things > like whitespace changes/comment fixes/etc where traceability isn't as > important. > https://developer.mozilla.org/docs/Mozilla/Developer_guide/ > Committing_Rules_and_Responsibilities > > I've come across uses of "No bug" commits recently where entire upstream > imports of vendored libraries were done. This is bad for multiple reasons: > * If makes sheriffing difficult - if the patch is broken and needs backing > out, where should the comment go? When it gets merged to mozilla-central, > who gets notified? > * It makes tracking a pain - what happens if that patch ends up needing > uplift? What about when that patch causes conflicts with another patch > needing uplift? What if it causes a regression and a blocking bug needs to > be filed? > > Bugs are cheap. Please use them. > > Thanks, > Ryan > > ___ > firefox-dev mailing list > firefox-...@mozilla.org > https://mail.mozilla.org/listinfo/firefox-dev > > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Please don't abuse "No bug" in commit messages
A friendly reminder that per the MDN commit rules, the use of "No bug" in the commit message is to be used sparingly - in general for minor things like whitespace changes/comment fixes/etc where traceability isn't as important. https://developer.mozilla.org/docs/Mozilla/Developer_guide/Committing_Rules_and_Responsibilities I've come across uses of "No bug" commits recently where entire upstream imports of vendored libraries were done. This is bad for multiple reasons: * If makes sheriffing difficult - if the patch is broken and needs backing out, where should the comment go? When it gets merged to mozilla-central, who gets notified? * It makes tracking a pain - what happens if that patch ends up needing uplift? What about when that patch causes conflicts with another patch needing uplift? What if it causes a regression and a blocking bug needs to be filed? Bugs are cheap. Please use them. Thanks, Ryan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform