4 ms·

That’s a bad process. The review of the feature should start before it’s merged into main branch, not after. It totally reasonable for people to be upset with a

That’s a bad process. The review of the feature should start before it’s merged into main branch, not after. It totally reasonable for people to be upset with a breaking feature in main branch without discussing it with the community at large. Changes merged in like this are how long standing tool lose community support.

tsimionescu 3y ago
The review of the feature was happening in public on the mailing list. All those who contributed to that review had their concerns addressed, and the change was merged into the main branch (which is the development branch of Emacs). Only afterwards did others complain.
- jbluepolarbear 3y ago
  The article says otherwise. It says that many concerns were disregarded during that time.
  - heyoni 3y ago
    Plenty of comments here saying that the change did more than make the feature opt in which is why it was rejected.
  - yaantc 3y ago
    The article is just his author view. Please read the emacs mailing list threads to get the full picture. GP is correct, and this is quite normal: some people using Emacs master will not follow all the mailing list discussions and commits. So they will notice a change only after it is merged. Nothing wrong with this, and nothing wrong with being unhappy about such a change. What's wrong in my book is the nature of the reaction show in this article (see my comment in reply to @tarsius).
    - kazinator 3y ago
      Then there are people who don't follow master. Some people only pick up releases, including test releases. Some people only work with final releases. All those people could find something suddenly not working well.
      - Kalq 3y ago
        If you are this hypersensitive to change, it behooves you stay on a stable release and only upgrade after you've read the CHANGELOG and NEWS files.
        kazinator 3y ago
        Someone hypersensitive to changes track changes as early as possible, and complain quickly and loudly. Those who are not sensitive changes just use whatever Ubuntu (or whatever) provides and are unconcerned with the development.
  - tsimionescu 3y ago
    That's what the article says, but I checked the actual mailing lists. Any replies that asked for a change of this kind came in after the patch was finalized. Even this author's own complaints. And, the author in particular didn't understand, or seek to understand, why the patch was added in the first place. Instead, their proposed "fix" that they complain about in the article reverted all of the improvements the author and reviewers made. Upon being informed of this, they simply complained that those are useless changes: > Indeed, I only reimplemented the parts I saw as clearly beneficial. Most importantly, my patch improves stuff without breaking other stuff. > Perhaps you can explain your use case for the rest of the changes, and if there's a good and compatible way to add them I'll be happy to look into it at some point. > > - No filtering. > > - No navigation. > > - No default registers. > > - No possibility to configure a new command added to register. > If you could elaborate about these bullets, and explain their use, that'd be great. I think it's quite obvious that engaging further with someone who came in with this attitude after a review was finalized (after ~5-10 rounds of feedback, I should point ou) and a patch applied did not seem worth it.
    - Y_Y 3y ago
      Perhaps you could mention what these improvements are or give a link to the messages you're getting this information from?
- gray_-_wolf 3y ago
  > into the main branch into the master branch
kazinator 3y ago
On the job, I wouldn't go implementing whatever pops into my head without input from my manager and product management. Even if the change could easily pass a technical review and get merged. First you have to determine: do we need this for the product? If this change is made, does it break user workflows? What difficulties will users have if they pick up this change?
- tsimionescu 3y ago
  Emacs is a community-driven project. And the author of this patch identified several issues with the feature, created a fix, proposed it for review, debated the impact with other devs on the mailing list for weeks, addressed all concerns raised, and then finally other more senior devs merged the change to master, where all new unstable changes go. Then, people who agree to test out all changes to Emacs by using the head of master found, basically the QA department of Emacs, came back with feedback that they like some of the improvements, but that they don't like one particular aspect (the extra RET). Thierry started addressing them, and a setting to disable the feature was added - but the initial version missed the mark. The article author also was part of this process, and even proposed a patch, but their attitude made others ignore it after the initial review (their patch reverted all changes and only implemented a tiny subset of the original, behind the discussed flag).
  - kazinator 3y ago
    I've now read (or at least skimmed) this in it entirety: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=66394 https://debbugs.gnu.org/cgi/bugreport.cgi?bug=66394 Thierry opened this as a simple bug, but then implemented something with multiple enhancements rather than addressing the bug, while causing UX regressions. It looks like only one developer was engaging the content, a Michael H., and he pointed out early, as far back as in October, both the problems that Eschel later latched onto. Eschel came up with a patch that just addresses #66394 in a small way. The reply to that was: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=66394#201 https://debbugs.gnu.org/cgi/bugreport.cgi?bug=66394#201 "But your patch only fixes this bug, reverting half a dozen features mine adds." Using a bug ticket as a vector for introducing enhancements is a software engineering no-no. They should merge the simplest patch that closes the bug without regressing anything or introducing extraneous enhancements. For those, a new enhancement ticket should be opened. If Eschel's patch can close #66394, and not break anything, the consideration of that it doesn't implement another solution's enhancements is actually a plus. There is possibly another bug ticket hiding in there based on the remark [y]ou reintroduced the old implementation which was not wrote correctly about handling various keys, particularly C-g. Existing behavior of not correctly handling various keys sounds like a problem different from the 66394 issue. If the project doesn't want the simplest fix for an issue, but to address something architectural, like with a view for adding enhancements or whatever, that can be turned into another ticket where you articulate that. The original bug can then be marked pending or blocked by that with a note that we don't fix this until that one.

tsimionescu 3y ago

The review of the feature was happening in public on the mailing list. All those who contributed to that review had their concerns addressed, and the change was merged into the main branch (which is the development branch of Emacs). Only afterwards did others complain.

jbluepolarbear 3y ago
The article says otherwise. It says that many concerns were disregarded during that time.
- heyoni 3y ago
  Plenty of comments here saying that the change did more than make the feature opt in which is why it was rejected.
- yaantc 3y ago
  The article is just his author view. Please read the emacs mailing list threads to get the full picture. GP is correct, and this is quite normal: some people using Emacs master will not follow all the mailing list discussions and commits. So they will notice a change only after it is merged. Nothing wrong with this, and nothing wrong with being unhappy about such a change. What's wrong in my book is the nature of the reaction show in this article (see my comment in reply to @tarsius).
  - kazinator 3y ago
    Then there are people who don't follow master. Some people only pick up releases, including test releases. Some people only work with final releases. All those people could find something suddenly not working well.
    - Kalq 3y ago
      If you are this hypersensitive to change, it behooves you stay on a stable release and only upgrade after you've read the CHANGELOG and NEWS files.
      - kazinator 3y ago
        Someone hypersensitive to changes track changes as early as possible, and complain quickly and loudly. Those who are not sensitive changes just use whatever Ubuntu (or whatever) provides and are unconcerned with the development.
- tsimionescu 3y ago
  That's what the article says, but I checked the actual mailing lists. Any replies that asked for a change of this kind came in after the patch was finalized. Even this author's own complaints. And, the author in particular didn't understand, or seek to understand, why the patch was added in the first place. Instead, their proposed "fix" that they complain about in the article reverted all of the improvements the author and reviewers made. Upon being informed of this, they simply complained that those are useless changes: > Indeed, I only reimplemented the parts I saw as clearly beneficial. Most importantly, my patch improves stuff without breaking other stuff. > Perhaps you can explain your use case for the rest of the changes, and if there's a good and compatible way to add them I'll be happy to look into it at some point. > > - No filtering. > > - No navigation. > > - No default registers. > > - No possibility to configure a new command added to register. > If you could elaborate about these bullets, and explain their use, that'd be great. I think it's quite obvious that engaging further with someone who came in with this attitude after a review was finalized (after ~5-10 rounds of feedback, I should point ou) and a patch applied did not seem worth it.
  - Y_Y 3y ago
    Perhaps you could mention what these improvements are or give a link to the messages you're getting this information from?
gray_-_wolf 3y ago
> into the main branch into the master branch

kazinator 3y ago

On the job, I wouldn't go implementing whatever pops into my head without input from my manager and product management. Even if the change could easily pass a technical review and get merged. First you have to determine: do we need this for the product? If this change is made, does it break user workflows? What difficulties will users have if they pick up this change?

tsimionescu 3y ago
Emacs is a community-driven project. And the author of this patch identified several issues with the feature, created a fix, proposed it for review, debated the impact with other devs on the mailing list for weeks, addressed all concerns raised, and then finally other more senior devs merged the change to master, where all new unstable changes go. Then, people who agree to test out all changes to Emacs by using the head of master found, basically the QA department of Emacs, came back with feedback that they like some of the improvements, but that they don't like one particular aspect (the extra RET). Thierry started addressing them, and a setting to disable the feature was added - but the initial version missed the mark. The article author also was part of this process, and even proposed a patch, but their attitude made others ignore it after the initial review (their patch reverted all changes and only implemented a tiny subset of the original, behind the discussed flag).
- kazinator 3y ago
  I've now read (or at least skimmed) this in it entirety: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=66394 https://debbugs.gnu.org/cgi/bugreport.cgi?bug=66394 Thierry opened this as a simple bug, but then implemented something with multiple enhancements rather than addressing the bug, while causing UX regressions. It looks like only one developer was engaging the content, a Michael H., and he pointed out early, as far back as in October, both the problems that Eschel later latched onto. Eschel came up with a patch that just addresses #66394 in a small way. The reply to that was: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=66394#201 https://debbugs.gnu.org/cgi/bugreport.cgi?bug=66394#201 "But your patch only fixes this bug, reverting half a dozen features mine adds." Using a bug ticket as a vector for introducing enhancements is a software engineering no-no. They should merge the simplest patch that closes the bug without regressing anything or introducing extraneous enhancements. For those, a new enhancement ticket should be opened. If Eschel's patch can close #66394, and not break anything, the consideration of that it doesn't implement another solution's enhancements is actually a plus. There is possibly another bug ticket hiding in there based on the remark [y]ou reintroduced the old implementation which was not wrote correctly about handling various keys, particularly C-g. Existing behavior of not correctly handling various keys sounds like a problem different from the 66394 issue. If the project doesn't want the simplest fix for an issue, but to address something architectural, like with a view for adding enhancements or whatever, that can be turned into another ticket where you articulate that. The original bug can then be marked pending or blocked by that with a note that we don't fix this until that one.