altMITgcm / MITgcm

M.I.T general circulation model
MIT License
1 stars 7 forks source link

Manual edits #17

Closed jrscott closed 6 years ago

jrscott commented 6 years ago

edits to contributing section, git workflow changes

edoddridge commented 6 years ago

@jklymak, @rabernat, and @jahn this is (potentially) the final version of the suggested workflow and contributors' guide to using git. It'd be great to have your opinions on it.

jklymak commented 6 years ago

I took a quick skim. Looks good.

The only issues I see going forward are

  1. your tags file. Matplotlib deals with this by having a next-whats-new directory where the user adds their whats-new entry as a next-whats-new/jmk-new-feature-2018-01-16.rst. Then they don't have to rebase/merge a guaranteed conflicting file. At release time, these are all concatenated (by hand or script, not sure) and given a new heading (Whats New 2.1.2).
  2. How are you going to handle releases? I appreciate you want to have some fluidity, and that seems fine, but do you really want all users working off Master? It kind of sounds like it, and that seems tough to maintain.

Anyway, I'm sure you already thought about both of these issues. Otherwise, looks good! Lots of work; thanks a lot for doing it!

rabernat commented 6 years ago

Regarding the "tags" / "whats-new" question: in xarray we just have a single master file in the docs called whats-new.rst. Every pull request needs to include an edit to this file describing the changes. We never have any issues merging this file.

rabernat commented 6 years ago

@jklymak is spot on about releases. I strongly encourage the use of github's release mechanism: https://help.github.com/articles/creating-releases/

The notion of releases syncs up roughly with our concept of checkpoints. Whenever we think it's time for checkpoint, we create a release. Readthedocs will also automatically manage the releases and build each version of the documentation appropriately. So users can be sure to have the correct documentation for the release they are actually using.

edoddridge commented 6 years ago

Thanks @jklymak and @rabernat!

Regarding doc/tag-index vs some other way of recording what's new: genmake2 parses doc/tag-index to find the latest checkpoint and puts that in the standard output. We can change that behaviour, but one of our aims is to avoid code changes as part of the transition to git.

As for releases, our (perhaps unstated?) intention was to use them like checkpoints are currently used, just as @rabernat suggested.

edoddridge commented 6 years ago

LGTM

jahn commented 6 years ago

I put the table example back for all builders, converted to just a set of simple tables, so it doesn't break the latex builder. LGTM