eclipse-platform / .github

Common contribution content for eclipse-platform repositories
https://www.eclipse.org/eclipse/
5 stars 10 forks source link

More Details on the recommened contribution workflow #120

Closed BeckerWdf closed 3 months ago

BeckerWdf commented 1 year ago

I just did unintentionally pushed my proposed change to the CONTRIBUTING.md directly to the main branch instead via a PR. Sorry for that.

See : https://github.com/eclipse-platform/.github/commit/ad7a5ef3eca83368ff47fce5f656e205d097846b

That's what I did:

Add steps that explain how to also add the upstream repository to the local clone. This explains how one can get the local clone back "up-to-date" after a pull request was merged into upstream.

Is this change ok?

merks commented 1 year ago

It looks fine to me.

I should note though that I don't personally find this setup quite ideal. The automatic Oomph setups clone the original repository and then it's easier to add your personal fork as an additional remote rather than the other way around. To my thinking this has the major advantage that when I switch back to the master branch locally and I do a pull, it pulls from the real original repository not from the master branch of my fork. Therefore I do not need to sync the master branch of my fork manually with the master branch of the original repository because nothing ever works with the master branch of my fork, e.g.,

image

Not having to worry about syncing my forks via the web page, especially when I have two dozen repos in the workspace, is a big win...

BeckerWdf commented 1 year ago

and then it's easier to add your personal fork as an additional remote rather than the other way around

Ah ok. Didn't know that. But the existing description already started with cloning the fork and not with cloning the "upstream" repo. So I continued on that road.

HeikoKlare commented 1 year ago

Thank you for updating the workflow description, @BeckerWdf! The changes also look fine to me.

I personally also prefer the workflow mentioned my @merks. But finally that's a matter of personal preference. Maybe we can split the description into two parts:

  1. The specific process for the eclipse-platform organization repositores: How to provide a contribution (PR from fork) together with how a PR is processed here (required checks/reviews/verifications, fetching PRs with EGit, preferences regarding one or multiple commits to rebase/squash etc.)
  2. The general options for workflows of providing a PR using GitHub (independent from the eclipse-platform organization): Creating and managing a fork, options of having fork or original repo as default remote etc.

The benefit I see from splitting up the information is that (1) is revelant for every (new) contributor, while (2) is only relevant for those new to Git/GitHub workflows. So for those who are already familiar with Git workflows, it would become easier to filter the relevant information for them.

Another point for which there does not seem to be a common agreement across all eclipse-platform repositories is;

12. If you push more commits to the same branch in your fork, they automatically get added to the pull request (and trigger a new round of builds and reviews).

At least in the eclipse.platform repository it seems to be common to amend commits to have a single commit per PR. The CONTRIBUTING.md indicates that you can make multiple commits, which eventually become squashed on merge. I also prefer the option to squash on merge (if reasonable), but independent from personal preference I think this is something all repositories should agree on, so that no unexpected comments are made in PRs that conflict with the general documentation for making contributions in the organization.

BeckerWdf commented 1 year ago
  1. The specific process for the eclipse-platform organization repositores: How to provide a contribution (PR from fork) together with how a PR is processed here (required checks/reviews/verifications, fetching PRs with EGit, preferences regarding one or multiple commits to rebase/squash etc.)
  2. The general options for workflows of providing a PR using GitHub (independent from the eclipse-platform organization): Creating and managing a fork, options of having fork or original repo as default remote etc.

That's a good idea.

HeikoKlare commented 1 year ago

Is there anything to add to this issue or may we close it after https://github.com/eclipse-platform/.github/commit/ad7a5ef3eca83368ff47fce5f656e205d097846b and #135?

BeckerWdf commented 1 year ago

Is there anything to add to this issue or may we close it after ad7a5ef and #135?

@sratz: That question goes to you. You have opened the initial issue.

BeckerWdf commented 1 year ago

I think regarding "recommened contribution workflow" this is fine.

mickaelistria commented 1 year ago

Sorry, I'm late in this discussion, but I find much of the new documentation is more or less duplicating documentation that is already provided by GitHub. IMO, it's better to reference external documentation, as it's more likely to be up-to-date and also it demonstrates that the project uses relatively "standard" workflow, which many users already know.

HeikoKlare commented 1 year ago

I agree, maybe the change moved the documentation into a wrong / too verbose direction. The recent change was motivated by the fact that the previous documentation proposed one specific workflow although there are multiple, equally valid ones within the standard fork-and-pull workflow. But it is true that there is now too much information that is generic / independent from this organization, and thus probably better maintained at some more "central" place.

For example, GitHub provides quite some information about the fork-and-pull model, different aspects of working with forks in general, and creating pull requests. So it makes sense to have another iteration replacing the generic information with proper links to maintained documentation and only keep what is specific to the workflow in this organization.

HeikoKlare commented 3 months ago

I finally found the time to come back to this discussion on the level of detail / verbosity of the contribution workflow documentation in the CONTRIBUTING.md. I have streamlined the information about the PR creation in #196. It now points to the standard workflow as described in the GitHub documentation and reduces most of the additional information to what is specific for the Eclipse (platform) process.

I am not sure about the information given in the section on creating and managing a fork. I do not see any information in that section that is specific to this project. Everything can, in theory, be derived from the standard fork-and-pull model that we also reference to. Still I experienced in the last months that it was helpful for several new contributors to have such a condensed description of the process specific for the Eclipse platform to get started quickly, as other description are usually more general and verbose. So I do not think that we should remove that information completely. It may make sense to move this information into the wiki and link from the CONTRIBUTING.md to the wiki instead of having such a verbose CONTRIBUTING.md. But then we should have some agreement on the expected level of detail in the CONTRIBUTING.md and on what information we expect to be placed, e.g., the wiki.

HeikoKlare commented 3 months ago

@mickaelistria do you have any remaining concerns with respect to the current state and level of detail of the contributing workflow information? Otherwise I would propose to close this issue, as I do not see any directly addressable demands or proposals remaining.