Contributing Guidelines

[StephenBrown2]

In the interest of establishing our common practice re non automated pull requests, I think it would be good if we always have issues that indicate the problem one is trying to solve. Years later there is then more context as others (later versions of ourselves) can see the origin of a task:

1. (Issue) I set out to solve this issue. I.e. Aim.
2. (Pull request) I ended up approaching it (referenced Fixed/Closes issue) this way. I.e. Proposal, in context.

That way we have more context as sometimes the solution is just wrong - but the problem remains. Or what the problem (issue) stated was the case(problem), was found to be wrong in the focused solution (PR). When we are unlucky we see both were misguided - but in that case we have two points of references to guide us/others further.

Ergo I suggest we try in this repo, like almost all others in the Org, to follow the regime introduced by the project founder @schakrava to me originally as I began contributing here. Also helps with defining scope and assisting with identifying if scope creep has/or is about to occur.

Originally posted by @phillxnet in #4 (comment)

As this is no longer a solo project, having been donated to the Rockstor project, it should fall in line with the expected contribution rules for that project, as mentioned above. As of now, I believe those guidelines include:

• An issue created for every PR to link to (to track the "problem" separately from the "solution")
• Squashed (single-commit) PR merges

After reading https://rockstor.com/docs/contribute/contribute.html#shipping-changes , I am unaware if there are any others that should be listed (there isn't a testing branch here to base PRs off, or I would have added that as a criteria).

I would request that we

• Add a formalized list to CONTRIBUTING.md in the root of the repo
• Enforce squashed merges in the repo settings

[phillxnet]

@StephenBrown2 Thanks for opening this issue to help clarify the org expectations.

However I think:

• An issue created for every PR to link to (to track the "problem" separately from the "solution")

Is already covered in the larger https://rockstor.com/docs/contribute/contribute.html that you sight, but in an earlier section than the one you quoted:

https://rockstor.com/docs/contribute/contribute.html#making-changes

Assuming you have identified or created an issue to work on ... first ensure your local code fork is up-to-date by rebasing on upstream:

&

You can then start making changes in this dedicated issue-specific branch

Similarly (and following on directly from the above):

• Squashed (single-commit) PR merges

Is addressed like-wise in: https://rockstor.com/docs/contribute/contribute.html#making-changes

We strongly encourage the following commit guidelines. As a guiding principle, separate your changes into one or more logically independent commits. This can help with the review process but only do this on larger more complex contributions. Otherwise we advise to squash your local ‘working steps’ commits into a single commit before submission.

Which more accurately covers our general guidelines in our canonical reference for the Project/Org. We also tend not to want or force squash on say commits resulting from reviews as then we loose the all-important attribution. That can get out of hand however, so we have to be pragmatic to an extent.

Re:

(there isn't a testing branch here to base PRs off, or I would have added that as a criteria).

Not yet anyway. Nor was there in any of our other repositories until they were required to build our stable release off of. And then only years after we initially established our stable - testing updates channels. The testing branches in most of the other repos (but note not in rockon-registry) were spun-off to enable larger breaking changes to be enacted while still being able to maintain a stable release (built from master (core-jslibs-rpmbuild)) where we have only more minor/pressing updates - if need be. An example of this is what we are currently doing in testing branch in rockstor-core - addressing major dependency updates i.e. Py2.7 to Py3.* Django updates etc. Those types of changes have not yet faced us in rockon-registry or here, or docs, or website etc.

[ ] Add a formalized list to CONTRIBUTING.md in the root of the repo

This, given our existing canonical reference in docs, already referenced by yourself, and me to your more detailed points, would seem redundant/anomalous. And we would then have multiple sources of truth, per-repo! However a reference to our contributor docs section within the README.md in this repo is a good idea: but not what you are suggesting with this issue, or point - but would make for an agreeably spin-off :) . And a more obvious omission/anomaly here I think.

We have this same reference in the README'S of most of our other repos e.g.:

• https://github.com/rockstor/rockstor-core#contributing

Development environment setup and contribution guidelines are available in our docs: Contributing to Rockstor - Overview

There is no Contributor License Agreement (CLA).

• https://github.com/rockstor/rockstor-rpmbuild#contributions

As with all 'The Rockstor Projects' repositories, contributions are always welcome. We depend upon domain experts to assist with our community endeavour. For contribution guidelines, see: Contributing to Rockstor - Developers.

• https://github.com/rockstor/rockstor-doc#rockstor-doc

For contribution guidelines, see Contributing to Rockstor documentation

• https://github.com/rockstor/rockstor-website#contributions

For contribution guidelines, see: Contributing to Rockstor documentation.

--

However as a counter example we currently have, in the more closely related (to this repo) rockon-registry:
https://github.com/rockstor/rockon-registry#one-time-setup-before-opening-pull-requests
&
https://github.com/rockstor/rockon-registry#steps-to-contribute-a-rock-on-with-a-pull-request

Which I see as anomalies actually (i.e.: their content could be moved to docs and referenced). And are likely also redundant. It may be we can reach a consensus that we have an existing outlier in rockon-registry re it's failure to link to a dedicated or related section within our contributor docs (which already exists)! And to save you the trouble of copying and pasting this, as a suggestion :), I've created the following issue:
rockstor/rockon-registry#350

and a partner issue in the interests of this same aim, across the org, in our rockstor-jslibs repo:
rockstor/rockstor-jslibs#35
Which looks to be another anomaly in this regard.

So given the above spin-offs and the suggested one within this repo (if you care to create that), I suggest that this issue/proposal be closed as having spun-off said issues to address an org wide consistency re NOT using CONTRIBUTING.md, but referencing within README.md contributor specifics where our existing doc section is canonical. There is a great deal of overlap between all our repos in current org practice; i.e. no contributor license agreement, Issue/PR(from issue-referenced-branch) sets etc so rather than increasing the inconsistency we already have, we instead reduce it: as per the spin-off issues indicated (and influenced by this issue) and the one requested here re enhance the README.md re Contributing. This would also help when we come to offering doc/website translations for example. Something that is considerably more difficult within a README.md or the like.

So hopefully this is a move forward and has definitely helped with the cited spin-offs. But I am not in agreement with the original proposals within this issue. However it has highlighted the detailed anomalies (now with associated issues): so all good hopefully.

Thanks again for your eyes on the Org - as always much appreciated.
And maybe there is also a further doc improvement (read issue/pr) to be had re detailing the org expectation/aim re perceived meaning/purpose of issue and pr text - such as you quoted in this issues original text.

[phillxnet]

Installer repo got left out in the cold - fixing now via:
rockstor/rockstor-installer#141

What a lot of repositories we now have !!