Closed wking closed 10 years ago
My motivation for the separate branches for swc-windows-installer.py and the Inno stuff is that I can test the swc-windows-installer.py without a local Windows box, so I can continue maintaining the script. That way changes to the script aren't blocking on Inno testing, and changes to Inno packaging aren't blocking on my review.
It doesn't seem to me that there should be many (or any) issues with these getting in the way of each other. The Inno script just packages the current Python script, and it will only be run when packaging a release, so everything should be stable. They Python script can easily still be run on its own. Given the unlikelihood of any issues I think the simplicity of having everything 'master' is worth it.
With respect to the namespacing issue, I have no objection to keeping the current 'master'/'namespaced' setup, but I have to admit that I don't fully follow the need for it now that we're moving development out of 'bc'.
On Mon, Jun 16, 2014 at 12:23:24PM -0700, Ethan White wrote:
It doesn't seem to me that there should be many (or any) issues with these getting in the way of each other.
Heh, I think I'm coming from “these are weakly coupled, so why develop them in the same branch?” and you're coming from “these are weakly coupled, so why bother with multiple branches?” ;). I expect the maintenance burden of both approaches to be similar on this side, and having separate branches is easier for consumers. For example, my non-bc aggregation 1 doesn't need the Inno stuff (which I can't build). If there's a branch without the Inno stuff, I don't have to do any cherry-picking to keep my aggregation up to date.
With respect to the namespacing issue, I have no objection to keeping the current 'master'/'namespaced' setup, but I have to admit that I don't fully follow the need for it now that we're moving development out of 'bc'.
Ah, you're suggesting that everything under setup/windows-installer/
be removed from BC? I was expecting BC's setup/windows-installer/
(at least in gh-pages
) would stay with both the source and the
compiled installer. That would allow gh-pages to use relative links
in the installation instructions, avoiding issues like
swcarpentry/bc#488 / swcarpentry/bc#496 / swcarpentry/bc#503.
On Mon, Jun 16, 2014 at 2:05 PM, W. Trevor King notifications@github.com wrote:
Heh, I think I'm coming from “these are weakly coupled, so why develop them in the same branch?” and you're coming from “these are weakly coupled, so why bother with multiple branches?” ;). I expect the maintenance burden of both approaches to be similar on this side, and having separate branches is easier for consumers. For example, my non-bc aggregation [1] doesn't need the Inno stuff (which I can't build). If there's a branch without the Inno stuff, I don't have to do any cherry-picking to keep my aggregation up to date.
In practically all cases the actual file that is going to be used in workshops is the binary installer that is compiled using Inno. Therefore I personally find it quite confusing that the 'master' branch doesn't have this material and that as a result the README needs to focus on using the Python script. I think walking potential new contributors through the structure of the repo will be confusing and I personally expect it to lead to more branching related errors on my part because it is a non-standard way of thinking about things (at least for us lowly scientists). A simple, intuitive repo, with everything in the 'master' branch seems worth the cost of letting a few extra setup files into your version even though you won't use them.
Ah, you're suggesting that everything under
setup/windows-installer/
be removed from BC? I was expecting BC'ssetup/windows-installer/
(at least ingh-pages
) would stay with both the source and the compiled installer. That would allow gh-pages to use relative links in the installation instructions, avoiding issues like swcarpentry/bc#488 / swcarpentry/bc#496 / swcarpentry/bc#503.
We're already linking to an external file for the installer (currently at files.software-carpentry.org) and I think it's better to just have one SWC version of this code. Part of my thinking with moving to the separate repository is that we can now use GitHub releases for managing new installer releases instead of needing to go through the process of emailing Greg the file so that it can then be posted on files.software-carpentry.org.
On Mon, Jun 16, 2014 at 01:45:47PM -0700, Ethan White wrote:
In practically all cases the actual file that is going to be used in workshops is the binary installer that is compiled using Inno. Therefore I personally find it quite confusing that the 'master' branch doesn't have this material and that as a result the README needs to focus on using the Python script.
The Inno branch (which may end up as 'master') can tweak the README to talk about how you're supposed to build and use the Inno installer.
I think walking potential new contributors through the structure of the repo will be confusing and I personally expect it to lead to more branching related errors on my part because it is a non-standard way of thinking about things (at least for us lowly scientists).
From the 'inno' branch it's just 'git merge master'. What sort of branching errors do you foresee?
A simple, intuitive repo, with everything in the 'master' branch seems worth the cost of letting a few extra setup files into your version even though you won't use them.
I'm not worried about the disk usage ;). I'm worried about being nominally responsible for files I neither use nor understand. I'd rather have those files in a separate branch with a separate maintainer ;). Of course, I'm happy to offload all of the maintenance/responsibility to you, and just stay on as a contributor. Changes to the script are not happening at such a tremendous rate that they need their own maintainer ;).
We're already linking to an external file for the installer (currently at files.software-carpentry.org) and I think it's better to just have one SWC version of this code.
That only works as long as everyone agrees on what needs to be installed ;). I'd rather focus on providing generically useful things, but keep it easy for others to build and deploy their own workshop-specific variants. Having relative links gives us one less thing that we have to keep globally stable, and gives instructors one less thing that they need to update if the roll their own installer.
On Mon, Jun 16, 2014 at 3:15 PM, W. Trevor King notifications@github.com wrote:
The Inno branch (which may end up as 'master') can tweak the README to talk about how you're supposed to build and use the Inno installer.
Yes, but that isn't the first thing that folks will see when they enter the repo so it looks like the repo is a single Python script that is run using Python, which is specifically what we've been moving away from over the last 6 months. Also, wouldn't this make merging more difficult (it would for people like me).
From the 'inno' branch it's just 'git merge master'. What sort of branching errors do you foresee?
I want to do some new work so I naturally checkout master and then create a new branch since that's what I do in almost every other repo. Then I get confused about why I'm missing half the files and have to start a new branch and move the work over. This isn't the end of the world, but since I'm often doing this stuff with 20 spare minutes it can be the difference between accomplishing something and not. The bigger issue here is that it won't be intuitive for the average person trying to contribute or just explore the installer and I think that's a problem.
I'm not worried about the disk usage ;). I'm worried about being nominally responsible for files I neither use nor understand. I'd rather have those files in a separate branch with a separate maintainer ;). Of course, I'm happy to offload all of the maintenance/responsibility to you, and just stay on as a contributor. Changes to the script are not happening at such a tremendous rate that they need their own maintainer ;).
Personally I think we can maintain this together with you being responsible for the .py script and me being responsible for Inno stuff. I'd even by happy to write that down in the README if it would make you more comfortable. That said, if it really makes you uncomfortable I am willing to take on sole maintainership if necessary since things have slowed down on the script side.
We're already linking to an external file for the installer (currently at files.software-carpentry.org) and I think it's better to just have one SWC version of this code.
That only works as long as everyone agrees on what needs to be installed ;). I'd rather focus on providing generically useful things, but keep it easy for others to build and deploy their own workshop-specific variants. Having relative links gives us one less thing that we have to keep globally stable, and gives instructors one less thing that they need to update if the roll their own installer.
My point here was that we aren't and haven't been using relative links for this anyway and we specifically decided against including the compiled installer in the repo itself so it's not possible to do so anyway in any standard case.
On Mon, Jun 16, 2014 at 03:12:09PM -0700, Ethan White wrote:
Mon, Jun 16, 2014 at 3:15 PM, W. Trevor King:
The Inno branch (which may end up as 'master') can tweak the README to talk about how you're supposed to build and use the Inno installer.
Yes, but that isn't the first thing that folks will see when they enter the repo
It would be if the Inno branch was 'master'. In fact, I just did that now ;).
From the 'inno' branch it's just 'git merge master'. What sort of branching errors do you foresee?
I want to do some new work so I naturally checkout master and then create a new branch since that's what I do in almost every other repo. Then I get confused about why I'm missing half the files and have to start a new branch and move the work over. This isn't the end of the world, but since I'm often doing this stuff with 20 spare minutes it can be the difference between accomplishing something and not.
With the current branch naming (Inno as 'master', the bare script as 'python'), that will work fine. I'll just rebase the PR onto the 'python' branch when I merge it, and then merge Python down into 'master'.
The bigger issue here is that it won't be intuitive for the average person trying to contribute or just explore the installer and I think that's a problem.
Maybe the 'master' rename addresses the exploration issue? And I'm certainly comfortable handling rebases for pull requests against 'master' that I think should have been against 'python'. As it stands, I've been performing similar cherry-picking from pull requests against bc, so this will certainly be a step in the right direction for me ;).
My point here was that we aren't and haven't been using relative links for this anyway
No time like the present ;). But…
and we specifically decided against including the compiled installer in the repo itself so it's not possible to do so anyway in any standard case.
Ah, I see that suggested in 1. Personally, I'd prefer it in gh-pages with relative links (adding an extra branch to keep the compiled version out of the local 'master'. With this setup, the branch-merge flow would look like:
-o---o---A---o python (developing swc-windows-installer.py) \ \ ---o---B-------C master (developing the Inno packaging) \ \ -----o-----------D compiled (compiled Inno executable) \ \ -------o-----------E namespaced (shifted into setup/windows-installer/) \ \ -----o---o-------F---G bc/gh-pages (released for workshops) / / ---o---o---o---o bc/master (lesson development)
Where:
That way every branch has a clear purpose, and can be completely ignorant of how downstream branches are using it. Folks can contribute to the 'python' branch without worrying about Inno, or to the 'master' and 'compiled' branches without worrying about Python. The 'compiled' and 'namespaced' branches can be automated with a release script, if maintaining them by hand is too burdensome. And everyone who wants access to the new script (including gh-pages) can opt-in by merging that 'namespaced' branch.
On Mon, Jun 16, 2014 at 04:02:11PM -0700, W. Trevor King wrote:
With this setup, the branch-merge flow would look like:
-o---o---A---o python (developing swc-windows-installer.py) \ \ ---o---B-------C master (developing the Inno packaging) \ \ -----o-----------D compiled (compiled Inno executable) \ \ -------o-----------E namespaced (shifted into setup/windows-installer/) \ \ -----o---o-------F---G bc/gh-pages (released for workshops) / / ---o---o---o---o bc/master (lesson development)
To make this more concrete, I just merged down a verb-conjugation fix:
$ git log --graph --topo-order --oneline --decorate --all
0b6b63e is a commit like A, 4b19750 is like B, e53be35 is like C, and cb49142 is like E. I don't have a commit like D or a compiled branch, because I can't compile the Inno executable locally.
And because GitHub can't be bothered to use a fixed-width font, here's the same graph from gitg:
And here's a fixed-width version of the hypothetical graph:
-o---o---A---o python (developing swc-windows-installer.py)
\ \
---o---B-------C master (developing the Inno packaging)
\ \
-----o-----------D compiled (compiled Inno executable)
\ \
-------o-----------E namespaced (shifted into setup/windows-installer/)
\ \
-----o---o-------F---G bc/gh-pages (released for workshops)
/ /
---o---o---o---o bc/master (lesson development)
Sorry for the delay. Part of it is that there's just a lot of complexity here for someone like me who isn't a git expert. To be honest I still don't fully understand what your suggesting and I don't have enough energy to dedicate to this for more explanation to help. So, what I have a few big picture points/suggestions:
bc
and it's own repo. That's just going to confuse people. If we think having it in bc
is really important then let's just go back to developing there.master
in the way we're used to, then I'm fine with keeping whatever additional complexity you want as long as you handle the extra work of merging/rebasing things properly into the extra branches.On Mon, Jun 30, 2014 at 08:47:23PM -0700, Ethan White wrote:
- I really don't think we want this in both
bc
and it's own repo. That's just going to confuse people. If we think having it inbc
is really important then let's just go back to developing there.
I think the development should be separate from bc, and I don't really mind if you want to bundle it into bc or not. If you're comfortable pushing everyone to a global files.software-carpentry.org installer, then that works for me ;).
- I continue to think that this setup is too complicated…. That said, since … this will allow …contributors… to simply PR against
master
in the way we're used to, then I'm fine with keeping whatever additional complexity you want as long as you handle the extra work of merging/rebasing things properly into the extra branches.
Done ;).
Awesome! Thanks. I'm very happy to have gotten to a good compromise so that we can get to work.
Continuing the discussion started earlier, I'm fine moving the Inno stuff into the master branch and having a
python
branch feeding intomaster
:python
:swc-windows-installer.py
namespaced-python
: mergingpython
with asetup/windows-installer/
prefixmaster
: mergingpython
with the Inno stuffnamespaced
: mergingmaster
with asetup/windows-installer/
prefixMy motivation for the non-namespaced branches is that we don't really care what namespaced the downstream consumer wants to use, so we might as well not use any at all, and leave it to other branches to setup preferred namespaces. Then we don't have noisy rename commits (like swcarpentry/bc@bf0ab1671) cluttering the source history, forcing folks who want these files to live in other locations to add their own ignore-rename commits to cancel the bc-initiated renames.
My motivation for the separate branches for
swc-windows-installer.py
and the Inno stuff is that I can test theswc-windows-installer.py
without a local Windows box, so I can continue maintaining the script. That way changes to the script aren't blocking on Inno testing, and changes to Inno packaging aren't blocking on my review.