Re: [sage-devel] Whitespace patchbombs
I'm against a workflow that involves bots for a nothing-burger like trailing whitespace. Either auto-cleanup on commit and have the server check on push that it was cleaned up. Or nothing at all. But waiting a day and then have a bot come back with trailing whitespace nonsense is just an unnecessary drag on development. -- You received this message because you are subscribed to the Google Groups "sage-devel" group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at https://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout.
Re: [sage-devel] Whitespace patchbombs
On 2018-02-20 21:47, Jeroen Demeyer wrote: > Interesting fact: the number of lines with trailing whitespace is > generally *increasing* with every Sage release. So it seems to me that > the biggest problem (if you find whitespace a problem) is preventing new > whitespace. So, there should be a trac plugin which checks if there are whitespace issues in the changed code lines or in the changed files... And, as it was mentioned in the other thread, there could be another trac plugin which runs pyflakes on the changed files. -- You received this message because you are subscribed to the Google Groups "sage-devel" group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at https://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout.
Re: [sage-devel] Whitespace patchbombs
How are merge conflicts handled, and is there any use for priority-flag on trac? It would make sense that lower priority tickets would be merged after more important ones. -- Jori Mäntysalo
Re: [sage-devel] Whitespace patchbombs
If I recall, when I originally setup the git-trac server, I included a hook that would reject/warn any introduction of new trailing whitespace. I believe this was a compromise made after a long discussion at a Sage days -- there were a lot of concerns at the time about the effort required to rebase mercurial patches with conflicts arising from a whitespace patchbomb. I also believe that we included in the documentation and server side hook a suggestion to use the trailing whitespace stripping commit hook (and there was a sage-dev command that would automatically do that, although sage-dev has now been removed in favor of vanilla git or Volker's git trac). On Tue, Feb 20, 2018 at 12:47 PM, Jeroen Demeyer wrote: > Interesting fact: the number of lines with trailing whitespace is > generally *increasing* with every Sage release. So it seems to me that the > biggest problem (if you find whitespace a problem) is preventing new > whitespace. > > > -- > You received this message because you are subscribed to the Google Groups > "sage-devel" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to sage-devel+unsubscr...@googlegroups.com. > To post to this group, send email to sage-devel@googlegroups.com. > Visit this group at https://groups.google.com/group/sage-devel. > For more options, visit https://groups.google.com/d/optout. > -- Andrew -- You received this message because you are subscribed to the Google Groups "sage-devel" group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at https://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout.
Re: [sage-devel] Whitespace patchbombs
Interesting fact: the number of lines with trailing whitespace is generally *increasing* with every Sage release. So it seems to me that the biggest problem (if you find whitespace a problem) is preventing new whitespace. -- You received this message because you are subscribed to the Google Groups "sage-devel" group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at https://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout.
Re: [sage-devel] Whitespace patchbombs
First of all, we should ensure that no new trailing whitespace is added. Otherwise, your effort is futile. The recently added file src/sage/rings/valuation/valuation_space.py is a bad example. I recall that we had a discussion about autogenerated patchbombs not so long ago. I think it's a bad idea because it can potentially generate a lot of silly merge conflicts. Why not remove it as part of the usual workflow, for example when refactoring something anyway? I generally don't mind if a ticket makes a bunch of whitespace changes (as long as it's not dominating the diff). By the way, the situation is not too bad: only 19 files have 20 or more lines with trailing whitespace. One file that stands out is functions/piecewise_old.py with 244 lines. -- You received this message because you are subscribed to the Google Groups "sage-devel" group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at https://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout.
