Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make a script to update python-novice-inflammation.zip #94

Open
tbekolay opened this issue May 10, 2015 · 7 comments
Open

Make a script to update python-novice-inflammation.zip #94

tbekolay opened this issue May 10, 2015 · 7 comments
Labels
good first issue Good issue for first-time contributors help wanted Looking for Contributors type:enhancement Propose enhancement to the lesson
Milestone

Comments

@tbekolay
Copy link
Contributor

We have a zip file in the main directory to make it easy to get access to the necessary files used in the lesson. There's repetition here, but not repetition that we can avoid for pragmatic reasons. But, we should make sure that this zip always contains the right versions of the right files, so it would be helpful to have a little Python or Bash script that either checks that this zip is correct, or perhaps ideally, creates the appropriate zip file (which should be identical to the existing zip file if the files in it are the same).

@iglpdc
Copy link
Contributor

iglpdc commented May 10, 2015

I'd like to have this as a tool at the lesson-template level, so it's available for other lessons too. Having some data files to download is more the norm than the exception.

It'd be nice if we could have this as a git post-commit script, so if a commit changes the data folder, a new zip is made automatically. :-)

@richford
Copy link

I was thinking about implementing this using git hooks with a script that goes something like:

#!/bin/sh
ZIP_FILE=python-novice-inflammation-data.zip
DATA_DIR=data
zip --filesync --recurse-paths $ZIP_FILE $DATA_DIR

Ideally, this would be a server-side hook (e.g. post-receive), but for very good reasons, GitHub won't allow us to execute arbitrary code on their servers as part of a server-side hook. So I think the remaining options are

  • Use a client-side hook. In order to put it under version control, we'd have to put the script in the main repository along with instructions to symlink it to .git/hooks/appropriate-hook-choice. Then we'd just have to trust that everyone has done so.
  • Dive further down the rabbit hole and attempt to implement this with a webhook, hoping that one of the supported webhooks (e.g. TravisCI or any of the other services I don't know much about) can be made to zip the data directory.
  • Just add the zip command to the Makefile as @iglpdc suggested in a shell-novice PR. Then the zip archive would update when anyone runs make preview.

I'd vote for the Makefile approach, but I don't really know much about webhooks so I might be overlooking a lot of the benefits. Anyone here have more experience with hooks?

@tbekolay
Copy link
Contributor Author

The client-side hook actually isn't too bad to do, as only the maintainers (me and @abostroem) need to actually have that hook installed. Makefile approach is good with me too, so either one IMO.

@richford
Copy link

For that matter, would the maintainers (@tbekolay and @abostroem) like make preview to be put in a client-side hook of some sort. My first reaction was that if you are going through the trouble of typing make preview before you commit, then it makes sense to just add the zip commands in my comment above to the makefile.

I can certainly submit a PR adding the zip commands to the makefile. Would that be done for python-novice-inflammation or for lesson-template? If it went in lesson-template, it seems that everyone would have to agree on the same zip file name (e.g. lesson-data.zip).

But this got me thinking: why go through the trouble of typing make preview at all? It seems like the lesson change workflow goes something like this:

  1. Someone makes a change to a lesson and submits a pull request.
  2. After approval, one of the lesson maintainers merges this request and commits the merge.
  3. The lesson maintainer then regenerates HTML and does another commit for that.

I think we could use hooks to get rid of step 3 above. The solution (borrowed from this thread) could look something like

  • Add .commit to .gitignore.
  • Add a pre-commit hook to indicate that a commit is taking place (this will make sense after viewing the post-commit hook):
#!/bin/sh
touch .commit
  • Then add a post-commit hook. If .commit exists then we know a commit just happened but the post-commit script hasn't run yet. We can use this to regenerate HTML files to the commit, and amend the last commit.
#!/bin/sh
if [ -f .commit ]
    then
    rm -f .commit
    make preview
    git add -u *.html
    # Amend previous commit using same message
    # Use --no-verify to bypass pre-commit hook so that we don't get caught in a loop
    git commit --amend -C HEAD --no-verify
fi

We could also use -c instead of -C to edit the commit message to add that we are regenerating html. Either way, we're left with only steps 1 and 2 from above.

I'm fairly new to SWC and don't really know anything about the maintainer workflow, so my apologies if this isn't a useful suggestion. What do you think? And should this be moved to the lesson-template repo? I'm not really sure if/how the lesson repositories talk to each other.

@iglpdc
Copy link
Contributor

iglpdc commented May 27, 2015

@richford, thanks for your suggestions. Some thoughts:

  • I think the machinery for this should be at the level of lesson-template, as most lessons have data files. Lesson-template is merged into the different lessons once in a while. The makefile is here
  • It would be nice to keep a different name for the zip files, as learners will download all of them to their Desktop. Could it be extracted in some automatic way?
  • Your commit-hook approach looks nice, but the problem is that many of our contributors don't have the tools to generate the html installed. If they try to do a commit with the hook in place, it'll fail in a weird way.
  • Note that the data files change don't change much.

@richford
Copy link

Thanks @iglpdc. I can open up a new issue in the lesson-template repo. Just to check, should this go in lesson-template or in lesson-example?

Either way, from your comments and from @tbekolay's comments, it seems like a good approach would be:

  • Keep the makefile as it is.
  • In the top-level directory, add a hooks directory with client-side hooks that will:
    • re-zip if necessary
    • regenerate html
    • add all these to the previous commit
      This would allow maintainers to adopt the git hook workflow or they could choose not to and keep doing things as is. That is, the client-side hooks would be tools that are available but not required to use.
  • Also include in the hooks directory a README.md with instructions on how to create symlinks in the .git/hooks directory and how to customize the scripts, e.g. in the python lesson:
ZIP_FILE=python-novice-inflammation-data.zip
DATA_DIR=data

The nice thing about having to symlink the client-side hooks is that contributors who don't want to regenerate the HTML wouldn't run into the dependency issues that @iglpdc mentioned.

Anyway, @tbekolay, @iglpdc, if you two like this approach, I'll go ahead and open an issue or submit a PR to the lesson-template repository.

@iglpdc
Copy link
Contributor

iglpdc commented May 28, 2015

Just to check, should this go in lesson-template or in lesson-example?

Lesson-template. This is the one with the tools we use across all lessons and gets merged periodically with them. Lesson-example has an example of the layout of the lesson and it's the one you should look to in you want to create a new lesson.

richford pushed a commit to richford/lesson-template that referenced this issue May 28, 2015
update zip files.

This enhancement grew out of
[issue
94](swcarpentry/python-novice-inflammation#94 (comment))
in the python-novice-inflammation repository and the discussion in a
[shell-novice PR](swcarpentry/shell-novice#141).
It creates a maintainer_hooks directory in the tools directory. Lesson
maintainers can use these hooks by symlinking to them in the .git/hooks
directory. The pre-commit hook sets a "flag" file called .commit and the
post-commit hook detects the presence of this flag and regenerates HTML
and updates the data zip file if necessary. It then amends the previous
commit by adding these files to the commit and keeping the same message.
richford pushed a commit to richford/lesson-template that referenced this issue May 28, 2015
update zip files.

This enhancement grew out of
[issue
94](swcarpentry/python-novice-inflammation#94)
in the python-novice-inflammation repository and the discussion in a
[shell-novice PR](swcarpentry/shell-novice#141).
It creates a maintainer_hooks directory in the tools directory. Lesson
maintainers can use these hooks by symlinking to them in the .git/hooks
directory. The pre-commit hook sets a "flag" file called .commit and the
post-commit hook detects the presence of this flag and regenerates HTML
and updates the data zip file if necessary. It then amends the previous
commit by adding these files to the commit and keeping the same message.
@fmichonneau fmichonneau added type:enhancement Propose enhancement to the lesson help wanted Looking for Contributors good first issue Good issue for first-time contributors and removed enhancement labels Jun 8, 2018
@ErinBecker ErinBecker added this to the June 2019 Release milestone May 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good issue for first-time contributors help wanted Looking for Contributors type:enhancement Propose enhancement to the lesson
Projects
None yet
Development

No branches or pull requests

6 participants