Dear MacPorts committers, I've seen a lot of commits to portfiles that do more than one thing. For example, a commit that updates the port to a new version, adds a new variant, and changes the indentation of the entire file. This makes it incredibly difficult to see what actually changed, and I would very much appreciate it if we could get into the habit of committing each logical change separately. For example, if you want to change the indentation of the file, do only that, then commit it, with a log message that says you're changing the whitespace. If you want to upgrade to a new version, do just that, and commit it with an appropriate log message. If you want to add a variant, do that, and commit it. This way it is much easier to look through the Subversion log and especially the diffs and see what was changed and why. If you do it all at once in a single commit, especially whitespace changes which like to affect every line, it's very difficult to see what was really changed, and this hinders people like me who are trying to learn more about MacPorts, portfiles, and compiling software in general. Hand in hand with the idea of one change per commit is the idea of accurate log messages. Your Subversion log message should say exactly what you changed. Don't omit anything! Just looking at some of the recent commits to the repository: http://trac.macports.org/projects/macports/changeset/22590 The log message says the port was updated to a new version, but also the maintainer was changed, which IMHO should have been a separate commit. http://trac.macports.org/projects/macports/changeset/22583 This revision updates the port version, changes the maintainer, changes whitespace, and adds the livecheck feature. I would have preferred separate commits, but at least the log does say that all of that was done. http://trac.macports.org/projects/macports/changeset/22543 The log message here says that livecheck was added, but also the software version was increased, which the log didn't say. http://trac.macports.org/projects/macports/changeset/22515 The whitespace of most of the file was changed, the maintainer was changed, the configure args were changed, and some patches were added, though it doesn't say why or what issue they were supposed to fix. I don't mean to be singling anybody out with the above commits; they're just some apt recent examples I found. I also should point out that I do most certainly thank all the MacPorts committers, since I am very much benefitting from all your hard work. But I not only use MacPorts to install software on my machine, I also like reading the portfiles to see how things were done, and especially, looking through the changes that were made to portfiles, and to read about why those changes were done. This is where one-change-per-commit and accurate (and, where appropriate, even detailed) log messages are so important, which is why I bring it up.
Ryan Schmidt wrote:
Dear MacPorts committers,
I've seen a lot of commits to portfiles that do more than one thing. For example, a commit that updates the port to a new version, adds a new variant, and changes the indentation of the entire file. This makes it incredibly difficult to see what actually changed, and I would very much appreciate it if we could get into the habit of committing each logical change separately.
For example, if you want to change the indentation of the file, do only that, then commit it, with a log message that says you're changing the whitespace. If you want to upgrade to a new version, do just that, and commit it with an appropriate log message. If you want to add a variant, do that, and commit it. This way it is much easier to look through the Subversion log and especially the diffs and see what was changed and why. If you do it all at once in a single commit, especially whitespace changes which like to affect every line, it's very difficult to see what was really changed, and this hinders people like me who are trying to learn more about MacPorts, portfiles, and compiling software in general.
Hand in hand with the idea of one change per commit is the idea of accurate log messages. Your Subversion log message should say exactly what you changed. Don't omit anything!
I totally agree on separate commits for logical changes in general, however, I'm of the mind that only whitespace change commits should be separate. I don't see the need to have separate commits for a new version and a new variant, after all, the developer may have tested them together, and testing them separately may be more work. Now if there's a complicated update and there's also other things that change, then yes, split them up. Regards, Blair
On 2007-03-05 19:27:05 -0600, Ryan Schmidt wrote:
For example, if you want to change the indentation of the file, do only that, then commit it, with a log message that says you're changing the whitespace.
Some changes will also change the indentation as a consequence (well, perhaps more rarely in Portfiles than in C code...). That's why diff (and Subversion, via -x) has the -w option to ignore all white space change in diffs.
http://trac.macports.org/projects/macports/changeset/22515
The whitespace of most of the file was changed, the maintainer was changed, the configure args were changed, and some patches were added,
You can see it more or less as a complete rewrite. There were outdated things and options that probably should never been used (but no comments saying why they were added).
though it doesn't say why or what issue they were supposed to fix.
Just look at the MPFR web site. The full description of what the patches fix would be a bit too much for the log file (IMHO, MacPorts lacks ChangeLog support). BTW, what homepage should be? The home page of the software or the home page of the particular version of the software? -- Vincent Lefèvre <vincent@vinc17.org> - Web: <http://www.vinc17.org/> 100% accessible validated (X)HTML - Blog: <http://www.vinc17.org/blog/> Work: CR INRIA - computer arithmetic / Arenaire project (LIP, ENS-Lyon)
Le 07-03-05 à 20:27, Ryan Schmidt a écrit :
Dear MacPorts committers,
I've seen a lot of commits to portfiles that do more than one thing. For example, a commit that updates the port to a new version, adds a new variant, and changes the indentation of the entire file. This makes it incredibly difficult to see what actually changed, and I would very much appreciate it if we could get into the habit of committing each logical change separately.
For example, if you want to change the indentation of the file, do only that, then commit it, with a log message that says you're changing the whitespace. If you want to upgrade to a new version, do just that, and commit it with an appropriate log message. If you want to add a variant, do that, and commit it. This way it is much easier to look through the Subversion log and especially the diffs and see what was changed and why. If you do it all at once in a single commit, especially whitespace changes which like to affect every line, it's very difficult to see what was really changed, and this hinders people like me who are trying to learn more about MacPorts, portfiles, and compiling software in general.
Hand in hand with the idea of one change per commit is the idea of accurate log messages. Your Subversion log message should say exactly what you changed. Don't omit anything!
Hi This sure makes sense, but it does not really fit with how a Portfile is worked. For example, I recently updated e17 stuff and had to change versions, dependencies, configure environment and arguments, and I finally decided to also opt in as maintainer. But there is no "logical order" to do this. 1 commit / change would have meant artificially creating many versions of the Portfiles afterwards with a few changes in each one. However, it is true log messages should be very clear as to what is happening. i.e. bump to version ... change configure.env ... change deps ... change configure.args ... change maintainer ... Lots of changes, but everything is written clearly. yves
On Mar 5, 2007, at 17:27 , Ryan Schmidt wrote:
I would very much appreciate it if we could get into the habit of committing each logical change separately.
That might work OK if the committer is the person making the changes, but if non-committers are to submit patches to Portfiles and patches (yes, patches to patches) via the wiki with one change per patch, then that creates a lot of extra work for the person making the patches and the person applying/committing the patches. It also leaves more room for error (forgotten patch, patches applied in wrong order, etc) than if the change (or changes) is (are) in one patch file.
Hand in hand with the idea of one change per commit is the idea of accurate log messages. Your Subversion log message should say exactly what you changed. Don't omit anything!
While I agree in principle, there is a fine balance to be maintained between completeness and conciseness. IMHO, the log message should be a comprehensive summary of what was done in the changeset, but not a verbose description that essentially duplicates the info that "svn diff" can provide. Just my two cents' worth, Dave
On Mar 5, 2007, at 11:22 PM, Yves de Champlain wrote:
Le 07-03-05 à 20:27, Ryan Schmidt a écrit :
Dear MacPorts committers,
I've seen a lot of commits to portfiles that do more than one thing. For example, a commit that updates the port to a new version, adds a new variant, and changes the indentation of the entire file. This makes it incredibly difficult to see what actually changed, and I would very much appreciate it if we could get into the habit of committing each logical change separately.
For example, if you want to change the indentation of the file, do only that, then commit it, with a log message that says you're changing the whitespace. If you want to upgrade to a new version, do just that, and commit it with an appropriate log message. If you want to add a variant, do that, and commit it. This way it is much easier to look through the Subversion log and especially the diffs and see what was changed and why. If you do it all at once in a single commit, especially whitespace changes which like to affect every line, it's very difficult to see what was really changed, and this hinders people like me who are trying to learn more about MacPorts, portfiles, and compiling software in general.
Hand in hand with the idea of one change per commit is the idea of accurate log messages. Your Subversion log message should say exactly what you changed. Don't omit anything!
This is a logical and sane petition by Ryan that we should all try to meet to guarantee the best communication possible between us all. However, as many of you have pointed out already, it is difficult to achieve and therefore a balance should be sought, chiefly what Yves points out below: sometimes one change per commit makes no sense and might even be impossible, sometimes that's exactly what the type of change mandates; all in all, I believe the moto should be "one logical block of changes == one commit", that logical block of changes be whatever the maintainer was working on as the next & inseparable feature set of the Portfile (version upgrade, version upgrade & new variants, just new variants & new maintainer promoting them, whatever), but no more than that. Needless to say, the log message should be as detailed as possible, but concerning only the changes made to the Portfile, not the ported (or upgraded, or revised, or whatever...) software. Citing the mpfr example given somewhere in this thread, a commit log detailing what each added patch does would be totally overkill; instead, a log simply listing the added patches and, if available, providing URLs to descriptions on each of them would be preferable. Regards to all and thanks for bringing this topic up, this is the type of thing we need to discuss as a group to improve our communication flow! All the best,... -jmpp
Hi
This sure makes sense, but it does not really fit with how a Portfile is worked. For example, I recently updated e17 stuff and had to change versions, dependencies, configure environment and arguments, and I finally decided to also opt in as maintainer. But there is no "logical order" to do this. 1 commit / change would have meant artificially creating many versions of the Portfiles afterwards with a few changes in each one.
However, it is true log messages should be very clear as to what is happening. i.e.
bump to version ...
change configure.env ...
change deps ...
change configure.args ...
change maintainer ...
Lots of changes, but everything is written clearly.
yves
_______________________________________________ macports-dev mailing list macports-dev@lists.macosforge.org http://lists.macosforge.org/mailman/listinfo/macports-dev
participants (6)
-
Blair Zajac
-
David MacMahon
-
Juan Manuel Palacios
-
Ryan Schmidt
-
Vincent Lefevre
-
Yves de Champlain