Closed davegwatson closed 7 years ago
Thanks for the PR, @davegwatson!
A few comments below; testing I will insist on, while the use of master
for development is optional and FYI, and if you're OK with the caveats and the remedy, we can continue with this PR using your master
branch. If you'd like to close this PR and open a new PR from a different branch, that's fine too. Let me know if you have any questions on either of these.
Could you please add some tests for your change? It's documented in the README
at the top-level and hopefully is easy to follow.
master
for developmentThis is a general comment and not specifically related to this repo, but in general, any GitHub repo.
You shouldn't in general add patches to your master
branch and open PRs from it: the idea is to keep master
match upstream, create per-feature branches, submit PRs from them, and once they're merged to upstream, delete local branches them, and update master
from upstream. By doing what you're doing, if we are to accept your PR, you will not be able to update your master
branch because it will be different than the master
branch on this repo — you'll have to delete your clones and start from scratch.
Sorry I missed the tests; they should be present now.
Thanks for the tip about master; I don't use github much, and Piper doesn't have that concept. I don't expect I'll be making tons of changes to this repo, but I will definitely keep it in mind for others.
FWIW, the master
/ per-feature-branch concept is probably only a thing in distributed VCS (e.g., Git, Mercurial, Darcs, etc.), simply because branches are so cheap and easy to create for anyone in their own clones, unlike with centralized VCS (e.g., CVS, SVN, Perforce, Piper, etc.) where branches are heavy-weight and hence created rarely, and always server-side.
Thanks for the contribution, @davegwatson! Best of luck with your upcoming launch.
I'm thinking of starting a list of projects which use autogen
; if you don't mind, please let me know when you launch and the URL to the project's repo.
I had to use MPL 2.0 for an upcoming OSS launch, and modified autogen to support that license.