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

GitHub Contribution Guidelines #130

Closed
dblodgett-usgs opened this issue Apr 18, 2018 · 79 comments
Closed

GitHub Contribution Guidelines #130

dblodgett-usgs opened this issue Apr 18, 2018 · 79 comments

Comments

@dblodgett-usgs
Copy link
Contributor

Dear All,

As a next step toward the CF community using GitHub tools to discuss and refine the specification, we need contribution guidelines for this repository. For background and interesting reading, this issue follows #106 and #112 and is more or less governed by the CF community conversation in Trac ticket 160. The consensus in that ticket has lead us here.

The outcome of this issue should be an initial CONTRIBUTING.md and modifications to the pull request template per the requirements described below. We should use this issue to discuss and evolve these requirements then I will submit a pull request with content according to the consensus.

Please review the use case and guidelines below. I will begin work on the CONTRIBUTIONS.md document in a few weeks based on the consensus we have sometime in May.

Top level use case: As a contributor to the CF specification, I need to know the rules and instructions for how GitHub should be used, so I know the right way to submit my suggested addition or changes.

Guidelines: (derived from correspondence between @JonathanGregory, @dblodgett-usgs and others.)

  • Some simple instructions are needed so that anyone who does not use GitHub for any other purpose can follow them successfully. GitHub can be used in a variety of ways and we need clear guidance regarding how the CF community wants to use the tools.

  • A given proposal should be discussed as one issue. It shouldn't fork or be superseded by another one, unless that reflects what has happened to the proposal, in the same way that we continue discussion under one trac ticket for a given proposal. This is so that it's easy to trace the discussion which led to a given agreed proposal.

  • It has to be easy to see what is being proposed in the context of the
    discussion. In Trac we say:

    I propose to do X, Y and Z, with the result that the text will read as follows
    in Section N
    blah blah blah
    and as follows in Section M
    rhubarb rhubarb

    Thus it's possible to see the reasoning and the effect on all the sections, all gathered together, and understand it. In GitHub, there is no relation between the argument and the modified text, unless the argument is put as annotations in the text, but in that case the bits of argument are all over the place. Somehow you need to get an overview of the changes all together.

  • In the case of short changes, I think that excerpting the existing spec in a GitHub issue with suggested new content in the body of a GitHub issue would work well. Once agreed to in the issue, the change can be implemented and submitted as a pull request referencing the issue.

  • In the case of long changes (i.e. where the body of an issue isn't long enough or changes are in many parts of the specification) the significant addition would likely come in the form of added content in a persons fork of the specification. They might register the contribution and their intent with the community in an issue for general feedback, work on the content in their fork, then submit a pull request when the consensus is that it's ready to be submitted. If the community finds utility in line by line comments in a pull request, that's great, but I don't think we need to force that issue. Line by line comments are really great for code. They can be a great for text, but they can also be very cumbersome.

  • Regarding "long changes" Maybe we could recommend that the discussion begins with proposed changes in the issue itself, but if the proposal involves changes in many places in the document, so it's awkward to spell them all out, it would be better to do this in a fork, but it's not worth doing that until it seems that the proposal is likely to be agreed. Moreover, if it does look like a proposal is going to be agreed, or when it has been, it would definitely be useful for the proposer to fork and update the documents, because otherwise someone else will have to do it later (as for trac tickets). So we should encourage that. However, to enable it, we need really simple instructions about how to do it!

  • Regarding a hybrid approach of GitHub and trac, it seems that part of that ship sailed when we started managing the text in GitHub. That said, I see no reason not to allow changes to be vetted in trac until we have enough success using GitHub to track modifications that the community is ready to leave trac behind. We are already referencing trac from GitHub. Opening the option to not use Trac seams like the next step to me.

Best,

Dave Blodgett

@JonathanGregory
Copy link
Contributor

Thanks for starting this, Dave. I'm grateful to you for taking it on - it think it will be very useful. Since you've already included all my comments (!) I don't have any more to make just now. I hope others do. Jonathan

@ChrisBarker-NOAA
Copy link
Contributor

Some thoughts:

Really great to get this out -- thanks for starting it!

Frankly, it's been a while since I've used TRAC much, but I think gitHub's Pull Request system provides some great opportunities to better manage changes to a document. IN the above, I see ideas like "Once agreed to in the issue, the change can be implemented and submitted as a pull request". But I think it makes more sense to go early with a PR. Often, the text itself what needs to be discussed -- so having the discussion directly in context of the suggested changes to the text has real benefits.

PRs support general discussion of the PR, as well as line-by-line comments specific text. This combination is ideal for many suggested changes to the doc.

So I suggest something like this:

Issues:

If there is a general issue about the standard that isn't specific to text, like "I suggest we support his new concept in CF" then that should be in an issue.

If someone notices and error, inconsistency, missing detail, etc, but does not have a suggested change to the text, that should be an issue.

Pull Requests:

In short -- go to a PR earlier rather than later -- as soon as someone has specific suggestions for changes to the text itself, that should go in a PR.

Any typo or minor correction should go straight to a PR.

PRs and issues can be pretty easily linked, so the transition is not bad -- if someone submits a PR that requires a larger discussion unlinked to any particular text -- it's not hard to start an issue and link to it. Similarly, if it starts as a issue, it's easy to link to a PR once one is created.

Potential Problems / Questions

Making it easy for newbies

Creating an issue is significantly less of a technical burden than a PR. That is, pretty much anyone with no experience with gitHub or text-based markup can probably figure out how to submit an issue. PRs, on the other hand, require more tech know-how -- gitHub makes it pretty easy to automatically create a fork for you if you try to edit a repo you don't have access to, but it's still a bigger lift to wrap your head around. So people should be encouraged to submit an issue if they have something to contribute (like correcting a typo, or...) that maybe should be a PR. A more experienced developer can make the PR later.

What do "core" contributors do?

People without write permissions to the repo need to create a fork to make a PR. But folks with write permissions to the repo have three choices:

  1. make a change directly and push it.
  2. make a branch, make the change in the branch, and submit a PR for the change
  3. make a fork into their personal account, and make a change and PR from there.

I personally prefer (2) as a core contributor to projects.

(1) should only be used for the simplest of changes like typos, etc.

Some folks prefer (3) as a more robust way to control the process, but It makes more work, and we're all volunteers here, so we want it to be as easy as possible to improve the doc.

In particular, (2) makes it much easier for multiple people to work together on a change -- rather than making PRs against forks againsts forks....

Just my thoughts, having worked on multiple gitHub projects, some large, and some small, some as core contributor, some not.

@graybeal
Copy link

Thank you Dave!

Well captured Chris, I concur. (And even for people who are newbies this could be a nice gentle introduction to Pull Requests!)

I may be misremembering, but I thought that given the right configuration, text files in GitHub can actually be 'edited in line' and submitted as a pull request. (I do know that if you have enough privileges you can edit in line and have the results merged straightaway, but for sure that's not what we're doing in this case.) That could be a nice path to describe for people, if true.

@dblodgett-usgs
Copy link
Contributor Author

Dear CF community,

I've started work on #130. It can be seen in pull request form between branches in my fork here or in rendered markdown form here

@ChrisBarker-NOAA -- I did not get all your ideas implemented. If you want to leave comments with proposed additional details or ideas for an additional section, I'm happy to copy/paste/rewrite, I just didn't really know how to fit your thoughts into the doc at the first pass.

Others, please don't hesitate to leave in-line comments on the pull request version, but please do also summarize your comments in line in this issue so they make it to the email list.

Note that I also added the Contributors Covenant as a placeholder for a code of conduct. Is there a pre-existing CF code of conduct written down some where?

@JonathanGregory -- do you want to act as moderator of this discussion? I think it would be in our interest to move it to near conclusion prior to the meeting in June.

Best regards,

Dave

p.s. apologies for the push of a couple commits to #115 that came over the email list a moment ago -- Those commits were promptly reverted and moved to another branch.

@ChrisBarker-NOAA
Copy link
Contributor

@dblodgett-usgs wrote (in https://github.com/dblodgett-usgs/cf-conventions/pull/10/files):

"""
My opinion is that Pull requests are really not a good format to evolve text like this. They are great for critiquing code and doing fairly tight reviews on style and syntax, but for long form review of text, they can be very problematic.
"""

My experience, like yours, is that PRs are great for small changes to text, fixing typos, syntax rewrites, etc.

And yes, not as good for long form review of text -- but I fail to see how an Issue is any better for long form review of text -- in fact, it's much worse -- and in particular, if you not adding a whole new couple pages, but rather one small section, to a doc (or a couple small sections that are related but in different parts of the doc), then you get as much context as you want, without an extra typing or management.

Now, for developing the first version of a largish doc with a small group, maybe Google docs is a better way to go, sure -- but I can't see how using Issues helps as all.

So again:

Once you are to the state of proposing a change to the text of the doc, whether it's a whole new section or a typo fix -- put it in a PR.

Also -- this is the first draft of the contributors doc -- whatever we do can be changed if things don't seem to be working as well as they might.

@dblodgett-usgs
Copy link
Contributor Author

Dear Chris,

Thanks for humoring me and working through this in the main repository issue. I commented back inline in my PR between branches in my personal fork of the repository, but will leave a note here as well.

Why issues for discussion and PRs for syntax review? Partly because @JonathanGregory requested that:

A given proposal should be discussed as one issue. It shouldn't fork or be superseded by another one, unless that reflects what has happened to the proposal, in the same way that we continue discussion under one trac ticket for a given proposal. This is so that it's easy to trace the discussion which led to a given agreed proposal.

And because I totally agree. A proposal should be described as a problem or deficiency in the current spec (an issue). Then the solution planned and discussed among the community as one conversation that is traceable in one place. I think the issue interface is very well suited to this for it's markdown capabilities and integration with email notifications, among other things. The ability to link an issue to a commit and/or PR that resolves it is a very nice way to trace the connection between a problem, discussion of the fix, and the actual implementation of the fix.

Why a PR over in my personal fork and not here? Two reasons, 1) so people can opt into the early line by line review / not inundate the cf email list with PR review spam and 2) to demonstrate what the interface is like without implying that we use it whole-hog. Per Jonathan and my observations, the PR interface is really not suited to commenting on sentences and paragraphs of text. It breaks the text apart and can quickly get out of hand. Look at how commenting works in google docs and word, that is much better suited to editing written word.

I hope that in the future, we have a nice interface for diffing, commenting, and evolving text for this kind of application, but right now we just don't. So we need to kick out to google docs or fork repositories in small groups to evolve the text of major contributions and look to the future for better solutions.


Rereading your and my comments, I think we are largely in agreement. I have a few minor changes I'll check into my fork in a moment. Maybe you could take my "CONTRIBUTING" branch and evolve the text to better convey the imperfect but pragmatic position we find ourselves in?

Best,

  • Dave

@JonathanGregory
Copy link
Contributor

JonathanGregory commented May 29, 2018 via email

@davidhassell
Copy link
Contributor

I would be very happy to act as moderator, if that OK with you, Dave?

Adding a label to an issue is easy and should go into the guidelines.

I agree with what has been said about googledocs, but we should remember that they are not, at present, universally accessible and so we should always have alternatives.

David

@ChrisBarker-NOAA
Copy link
Contributor

ChrisBarker-NOAA commented May 30, 2018 via email

@ChrisBarker-NOAA
Copy link
Contributor

@dblodgett-usgs wrote:

I hope that in the future, we have a nice interface for diffing, commenting, and evolving text for this kind of application, but right now we just don't. So we need to kick out to google docs or fork repositories in small groups to evolve the text of major contributions and look to the future for better solutions.

Frankly, gitHub PRs are a pretty darn good interface for "diffing, commenting, and evolving text" it does fall down a bit on a having a good way to comment on an existing doc, rather than just the diff, but not too bad.

"kick out to google docs" (or anything else) is a bad idea -- the whole point of git is to preserve the history, that's the part we really want to keep.

However, key is that we need to start with a policy, and then see how it goes -- frankly, we can't nail down an exact policy anyway, and people wouldn't exactly follow it if we did.

But in short, my suggestion is:

  • Use gitHub issues for managing general discussion

  • Use gitHub PRs for proposing and discusing changes to the text of the document.

  • Use branches when there is a large change, particularly if it effects multiple parts of the doc.

  • Use whatever the heck you want to develop the first draft of a major addition.

@dblodgett-usgs
Copy link
Contributor Author

Dear all,

Apologies for going mute for a bit. Apparently, my email overlords (bow to the security gods) decided that github notifications are spam -- amazing how you don't miss notifications when they are gone.

I 100% agree that an external document is an imperfect solution that is really only suited to pre-community-review draft development.

I also agree that we should have issue "labels" to enhancement, defect, and typo. I will add a note that labels should be used if at all possible to the guidelines.

Yes, @davidhassell -- sounds great to have you moderate.

@erget
Copy link
Member

erget commented Jun 20, 2018

I agree with the discussion wholeheartedly, which for the sake of keeping things tidy and confirming my own understanding I summarize as follows:

  • If you want to change something and are in doubt, raise an issue
  • For anything else, a PR is a good way to go, although an issue isn't harmful, especially if it's about a typographical matter
  • What I'm missing is a release strategy, which may need to be addressed elsewhere. I propose something inspired by Git Flow, roughly as follows:
    • master is the current release.
    • next is the next release, an "editor's draft", if you will.
    • If you want to make a change, make a branch based on next and implement it. Then send a PR to merge it back into next or ask for help.
    • When a change has been approved, it's merged into next.
    • When we make a release, we merge next into master and tag that.

Perhaps more refinement is possible - we discussed at the meeting having some kind of a beta version of a next release for a time. This would be compatible with this workflow (which is one of many possible ones, but fairly simple and IMHO straightforward).

@cameronsmith1
Copy link

This seems reasonable to me. I would expect that some active management may be needed in the run-up to a new release, ie managing which PRs get merged or held until after the release. However, that shouldn't affect your general outline.

@ghost
Copy link

ghost commented Jun 21, 2018

I propose something inspired by Git Flow, roughly as follows:
master is the current release.
next is the next release, an "editor's draft", if you will.

I don't think we need a next branch. Each new convention version will be a tagged release in the master branch thus allowing this branch to represent the Editor's Draft. All the new material merged into the master branch after the last release is assumed to be accepted for the next release, barring any stylistic or formatting changes.

The process would go something like this:

  1. Anyone who wants to propose a change to the convention should fork this repository. (I assume the number of those with commit privileges will be small so forking is what majority will have to do.)

  2. Create a branch off the master in the forked repository and work on the proposed changes in this new branch.

  3. Creating a pull request to the upstream (the official convention's) repository initiates the formal review process of the proposed changes.

  4. Pull request creator is responsible for updating its branch and the text of the changes with the upstream's master branch during the review process.

  5. The review process ends when the pull request branch is merged into the upstream's master branch.

It would be good to appoint, at least nominally, a few editors for every new convention release. They would be in charge of preparing the text in the master branch for the next release and making sure the review process of any open pull request is reasonably timely.

@dblodgett-usgs
Copy link
Contributor Author

I 💯% agree with @ajelenak-thg.

I think this is more or less how the repository works now and this is a very natural pattern. Released tags would get built and stored as binaries here: https://github.com/cf-convention/cf-conventions/releases as well as on the main cf web page.

@erget
Copy link
Member

erget commented Jun 22, 2018

This is definitely the most natural way to work on GitHub and very git-native, which I like. However, I do think there is merit in having a next branch. Because CF isn't software that we're releasing with continuous delivery, having fewer, but larger, changes between versions is probably the better way to go. It also better reflects the understanding achieved at the meeting - namely, that master is basically a release branch, and that the draft (whether named next or something else) collects changes before merging them all into master.

Concerning the workflow @ajelenak-thg outlines - that's certainly the way I intend to work, but my feeling is that we'll perhaps need to have a bit of patience with people who are new to both git and GitHub as they get used to it, so we shouldn't be too strict with enforcing the specifics of that workflow.

So the only point of ambiguity for me is the branches. I do think that having master only be merges with tagged releases has advantages for novices - even though you can achieve the same amount of traceability by only tagging releases, and declaring intermediate commits on master as transitionary, I think that having non-tagged releases appear as the repo's landing page could be confusing to some of our contributors.

@marqh
Copy link
Member

marqh commented Jun 22, 2018

I think that the people who are new to github will appreciate a lack of complication in the structure of information and how to engage.

Having a single branch makes is clear to anyone arriving that if they want to propose a change, that is what their change is with respect to.

So, I am in favour of maintaining the master as the editor's draft and providing tags for released versions and against a next branch.

I have also proposed a change to the CF website that makes the current state a little more clear: cf-convention/cf-convention.github.io#61

@ChrisBarker-NOAA
Copy link
Contributor

ChrisBarker-NOAA commented Jun 22, 2018

I agree with @ajelenak-thg and @marqh here -- I don't see a utility in having a next branch.

In general, master is supposed to be the "lastest" version, not ready for release yet.

As for "CF isn't software that we're releasing with continuous delivery, having fewer, but larger, changes between versions is probably the better way to go."

I'm not sure that it makes much difference how many changes there are between versions. Also -- maybe we should move to a more continuous delivery model -- why not have more frequent point releases that have clarifications and typos corrected?

What we should do is be sure to crate a branch for significant changes or additions, then they can be fully developed in that branch until ready for merging into master.

@erget
Copy link
Member

erget commented Jun 23, 2018

I don't feel strongly about how we do branches and since I'm the only one currently supporting having a next branch on here that's fine with me. I think the release frequency is something which should be discussed more widely, but it's independent of our workflow for producing releases.

@davidhassell
Copy link
Contributor

I think that we have to be very clear this is a proposal for how github issues can replace Trac tickets (as opposed to anything else we currently use, like the mailing list) and, at the this time at least, nothing else. This issue is concerned with how best to use github to carry this out in a way that make sense to people now, and in 20 years time.

In particular, github issues are only for completely defined proposals, not for any sort of speculative change; and all proposals must be raised as an issue (although that could coincide with a pull request for simple cases, e.g. typos).

The discussion has widened a bit to include "how should we integrate accepted proposals into CF", which is fine, but this is a separate concern to the original "how should we get a proposal accepted". These two are certainly connected, but will need quite distinct guidelines.

The original proposal contains guidelines that I think say

  1. Start by using a github issue just as you would a Trac ticket
  2. ...
  3. When it is finished there will be a pull request

How we get from 1. to 3. is less clear to me, but that will be resolved by the discussions in this issue.

When I come back in 2038, I need to be able to start at the top of #130 and be able to follow the arguments that led to its conclusion. Linking out to pull requests as part of this read through is fine by me, although I have a concern about searching - if I search for "foobar" in the issue, will it also search for that string in all referenced pull requests? This was also an issue for documents attached to/linked form Trac tickets, but the reality was that they were very rarely used, and if they were it was not for essential items such as proposed text changes.

@JonathanGregory
Copy link
Contributor

Dear all

As David H says, I think we should first decide the procedure for making decisions on changes using GitHub (as an alternative to Trac). This will be part of a modified version of http://cfconventions.org/rules.html, so at this point we need explicit text to put in that document. In principle it should be the same procedure as now regarding the time-limits etc.

One change in the procedure could be to recognise an extra kind of change request i.e. typo, which is different from defect or enhancement. A defect is a proposal to change the words of the document to correct an error or clarify them, with materially changing the meaning of it. Someone else might disagree with the proposer of a defect ticket and think what they propose is actually a material change to the meaning, and in that case the proposal has to be discussed as an enhancement instead i.e. not accepted by default. A typo is an even more minor change which fixes something that seems evidently to be simply a mistake. However it's possible, in an analogous way, that someone else might say that it's not a typo, but deliberate, in which case it'd have to be discussed as a defect or enhancement instead.

I think that a typo could be proposed with a pull request, but a defect or an enhancement should be started with an issue, as David H says, and proceed to a pull request when it's fairly well agreed and it comes to a matter of wording.

Cheers

Jonathan

@davidhassell
Copy link
Contributor

I have a concern with the hybrid github/trac approach: Some github issue numbers are/will be the same as Trac ticket numbers.

Is this managable?

David

@ghost
Copy link

ghost commented Jun 27, 2018

It is manageable only with additional context, for example: Trac #101 or GH #101.

Alternatively, GitHub's API can be used to create and close dummy issues until the internal numbering is bumped up above the highest ticket number on Trac. Or go all the way to, say, 200. So all ticket numbers below 200 belong to Trac; above 200 to GitHub.

@ChrisBarker-NOAA
Copy link
Contributor

ChrisBarker-NOAA commented Jun 27, 2018 via email

@sebvi
Copy link

sebvi commented Jun 27, 2018

why not simply import all past TRAC tickets and convert them into github issues?
I am sure we can find that kind of scripts, possibly in a github repo!

@sebvi
Copy link

sebvi commented Jun 27, 2018

1mn googling: https://github.com/mavam/trac-hub

@marqh
Copy link
Member

marqh commented Aug 14, 2018

i agree with @ChrisBarker-NOAA

I am in favour of instructing contributors to propose all changes to master and tagging releases as they are created.

Branches are not required for managing the document in its normal work flow (though they may be useful for occasions such as where an updated 'point release' is required to an existing release).
This is for administrators of the repository to manage, not for contributors to have to negotiate.

I think that managing branches for each 'next version' will cause maintenance problems and confusion for contributors.

There are already loads of branches within this repository, which can/will lead to further confusion, especially if we ask contributors to understand why these branches are there and which one to pick.

I recommend keeping the use of branches limited to repository administrators and pointing all contributions and contributors at master.

I also recommend ensuring that accepted changes are merged to master as soon as they are approved, so that further changes are being made with respect to the latest approved changes.

I am fairly clear on the benefits of this approach and concerned about the risks of pointing contributors at branches and holding off merging agreed changes to some future time.

@ChrisBarker-NOAA
Copy link
Contributor

@hrajagers wrote:

The branch per "release" approach might make it conceptually a little bit easier to start working on a draft of "release 2.0" while also working on an earlier "release 1.8".

Agreed -- master should be for the current "working version" -- i.e if 1.8 is out, changes that will be in 1.9 go in master.

In the usual case of software, you have a "current release", say 3.2 (choosing examples that aren't currently used for CF), and you are working on 3.3 in master.

Meanwhile, if you are maintaining older versions, say 3.1.3, you might do bug-fixes on that, and release a 3.1.4 -- in which case, there is a branch for 3.1.* where the bug fixes go.

In the CF case, we don't have to fix bugs in older releases, so it's easier.

The one complication is if we have a "future" version that wont' be released for a while -- that would be 2.0 for CF -- so that should be in a separate branch -- or maybe a separate repo altogether.

I jsut took a look at the cPython repo -- for an exampel of a major project:

https://github.com/python/cpython

The currently have:

2.7
3.4
3.5
3.6
3.7
master

3.7 is the latest version -- they are working on 3.8 in master, and 2.7--3.6 are in maintenance mode (i.e. just bug fixes).

@ChrisBarker-NOAA
Copy link
Contributor

There are already loads of branches within this repository, which can/will lead to further confusion,

Those look like "feature branches" -- which can be "pruned" once they are no longer needed.

https://railsware.com/blog/2014/08/11/git-housekeeping-tutorial-clean-up-outdated-branches-in-local-and-remote-repositories/

Note that if a branch has been merged, then pruning that branch doesn't lose any information in the history.

@dblodgett-usgs
Copy link
Contributor Author

I agree with @marqh but want to recognize that this should be, to some extent, at the discretion of the repository admins and issue moderators. I say this because a given change may have a different disposition with regard to the current document depending on the state the overall project is in at the time. e.g. we are working hard on v2.0 but v1.9 is about done. I wouldn't want to 100% pre-judge how people want to use branches for a given PR at that point in the future.

So --

  1. PRs get opened against master which should be even with or recognizably ahead the latest release.
  2. past releases are tags on master. No branch needed because we don't bug fix old releases.
  3. branches are used as short and/or long term feature development space.
  4. approved changes are merged to master as soon as possible after acceptance.

Then the question is, should we add something to this affect to CONTRIBUTING.md?

@ChrisBarker-NOAA
Copy link
Contributor

Sounds good -- and yes, this should go in CONTRIBUTING.md

@cameronsmith1
Copy link

We started using Github a few years ago for a project I work on. It was decided at the start that changes should go onto master frequently, and that we would simply tag master for the major release versions. It didn't work in practice, because near release time there were divergent needs within the project. Some people wanted to only allow changes to master that were for finalizing the release, while other people needed to keep working on future capabilities. If finalizing the release only took a couple of weeks this wouldn't be a big deal, but big releases often takes months to finalize (CF tends to spend a lot of time agonizing over the details too). I think this is the issue that @dblodgett-usgs mentioned in his last post (above). However, I don't see it explicitly mentioned in the 4 bullets.

There are two ways I can see this working near release time: (a) master is for the release version and future changes go onto one or more feature branches that get merged to master after the release, or (b) vice versa, ie the release on a branch and the master keeps developing. In my project we ended up doing (b), but it would probably have been better to do (a).

In any event, I think it would be good to add a fifth bullet to the list of @dblodgett-usgs that clarifies how master and branches will be used near a release.

@ChrisBarker-NOAA
Copy link
Contributor

@cameronsmith1: good points.

I think the way to do it is to crate a "release candidate" branch when getting close.

-CHB

@dblodgett-usgs
Copy link
Contributor Author

@cameronsmith1 -- my point on that was that I don't think any guidance we write now is going to be right. So we should use our best judgement on the workflow when we get there. I would rather not attempt to write rules for that stuff until we have more experience.

@davidhassell
Copy link
Contributor

There are clearly good reasons for making PRs against master, so I withdraw my suggestion of a using a branch.

I like 1, 2 and 4 of Dave's list (#130 (comment)) but don't see the need for point 3: "branches are used as short and/or long term feature development space.". This, I think, is not relevant to creators of PRs so we don't need to mention it in this document.

The same seems true for the management of near release time - we don't need this in CONTRIBUTING.md because it won't affect contributors, who would carry on making PRs to master, regardless of the release mechanics being employed at the time.

@dblodgett-usgs
Copy link
Contributor Author

Agreed - @davidhassell. My number 3 is more about how the sausage is made -- which we'll work out as things go forward.

I'll get the CONTRIBUTING.md updated soon.

@cameronsmith1
Copy link

Removing 3 from Dave's list certainly makes it clean and simple. The other issues can then be deferred until they are needed, per long-standing CF tradition.

@graybeal
Copy link

graybeal commented Aug 16, 2018 via email

@ChrisBarker-NOAA
Copy link
Contributor

"I am assuming here that 'someone who wants to create a pull request' may actually want to create a rather complex change and work on it a little while with other folks"

I do that that is a significatn use case -- but whether that is a feature branch or a PR from a fork depends on involved someone with permissions on the repo is.

If an "outsider" starts a PR against master, the repo managers can choose to merge that into a feature branch instead of master if they want to develop it for a while before merging.

In short -- creation of a feature branch is the job of the core maintainers, so I'm not sure it needs to be mentioned in Contributing.md

Though maybe should be mentioned somewhere else as part of recommended workflow.

@davidhassell
Copy link
Contributor

I looks like CONTRIBUTING.md is in good shape and, as I see it, there are no outstanding issues. If there are any further items to consider please post soon, as otherwise this issue will be accepted as is after the 3 week deadline is up. Thanks!

@cameronsmith1
Copy link

It looks good to me. I noticed a couple of typos:
+) In guideline 1, there is a 'tha' which should be 'that'.
+) In request 3, there is a 'their' which should be 'there'.

@dblodgett-usgs
Copy link
Contributor Author

typos corrected.

@davidhassell
Copy link
Contributor

Hello,

Three weeks have passed with no further discussion, so I think we can accept this issue - i.e. the new CONTRIBUTING.md file.

Very many thanks to everyone who gave their time and git experiences to this discussion on the mailing list, at the CF meeting in June and here on the github issue.

David

@dblodgett-usgs
Copy link
Contributor Author

Wonderful, thanks for working through this, everyone! This is a big step for the community. When we merge #137, we should also merge and deploy cf-convention/cf-convention.github.io#62.

@JonathanGregory
Copy link
Contributor

Dear all

That's very good. Thanks for proposing it, Dave.

David, would you be able to consider how and where to incorporate this in the governance page http://cfconventions.org/governance.html? Similarly, there is issue which Dave refers to about defects. How does these relate to trac ticket 160 and have we decided to stop using trac for new tickets? We need some clarity, I would say, so that proposers of changes know how to proceed.

Best wishes

Jonathan

davidhassell added a commit that referenced this issue Oct 12, 2018
[Issue #130] -- CONTRIBUTING

This PR arises from the concluded discussion on  #130
@dblodgett-usgs
Copy link
Contributor Author

Now that #137 is merged, I think we also need to merge: cf-convention/cf-convention.github.io#62 right? Once that's done, I think this issue should be closed.

@davidhassell
Copy link
Contributor

Absolutely right. It will be merged very soon ....

@ChrisBarker-NOAA
Copy link
Contributor

Can we close this now? not good to have lingering issues....

I've started #150 -- where we can hash out the issues not addressed in the CONTRIBUTING.md doc

@JonathanGregory
Copy link
Contributor

Dear Chris et al.
David Hassell has now got permissions to merge pull requests. I believe this will be done soon, now he is able to do it (since a couple of days ago).
Cheers
Jonathan

@davidhassell
Copy link
Contributor

Just got my access permissions sorted out. cf-convention/cf-convention.github.io#62 has been merged. Thanks

@dblodgett-usgs
Copy link
Contributor Author

With @davidhassell's merge, I'm going to close this as complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests