Skip to content

Comments

Update PR instructions in CONTRIBUTING.md#31

Open
jcailler wants to merge 1 commit intoGoelandProver:masterfrom
jcailler:contribution
Open

Update PR instructions in CONTRIBUTING.md#31
jcailler wants to merge 1 commit intoGoelandProver:masterfrom
jcailler:contribution

Conversation

@jcailler
Copy link
Member

Add git instruction for contributors

  • How to create a branch
  • How to create a pull request
  • Random useful tips

@jcailler jcailler requested a review from jrosain July 10, 2025 08:47
@jcailler jcailler added request:ci Requests a CI run from the workflow part:infrastructure The PR is on non-goéland code kind:documentation Enhance or add documentation labels Jul 10, 2025
@jcailler jcailler added this to the 1.2 milestone Jul 10, 2025
@jrosain jrosain removed the request:ci Requests a CI run from the workflow label Jul 10, 2025
@jrosain
Copy link
Member

jrosain commented Jul 10, 2025

FYI as the go source code has not been modified, we don't need to run the full CI on this PR.

(see e.g., on the maintainer section of the contributing file)

@jrosain jrosain added the needs:discussion The PR needs to be discussed label Jul 12, 2025
@jrosain
Copy link
Member

jrosain commented Jul 12, 2025

To be honest, I'm not sure it's our job to give these tips. I'm not against contributors having their cheatsheet, or even linking to a github command cheatsheet inside the contribution file, e.g., this one for instance.

My initial thoughts: I don't think we should keep the "create a branch" section, but maybe we can keep the creation of a PR / rebasing comments by factoring them through a specific section for pull requests. But the text would need to be reworked before. For instance, you should push your branch, then open a PR.
Squashing the commits is not mandatory as we might want to keep the history in some cases (see e.g., #26). We should precise that the maintainer will tell whether to squash the commits or not, so that you don't have to worry about that when opening a PR. We should definitely keep the rebase command and tell that before opening a PR, a branch should either be up-to-date from upstream/master or mergeable without conflicts (even though this is already said in the general informations, we could provide the commands).

@jrosain jrosain marked this pull request as draft July 12, 2025 12:11
@jrosain
Copy link
Member

jrosain commented Jul 12, 2025

I'm converting this PR to draft until further discussions / progress.

@jrosain jrosain removed their request for review July 12, 2025 12:12
@jrosain jrosain removed their assignment Jul 12, 2025
@jrosain jrosain removed this from the 1.2 milestone Jul 12, 2025
@jrosain jrosain added the needs:progress The PR needs to be updated w.r.t. the discussions label Jul 12, 2025
@jcailler jcailler requested a review from jrosain July 16, 2025 13:47
@jcailler jcailler assigned jcailler and jrosain and unassigned jcailler Jul 16, 2025
@jcailler jcailler added this to the 1.2 milestone Jul 16, 2025
@jrosain jrosain marked this pull request as ready for review July 16, 2025 16:49
@jrosain jrosain removed needs:discussion The PR needs to be discussed needs:progress The PR needs to be updated w.r.t. the discussions labels Jul 16, 2025
@github-actions github-actions bot added the needs:ci Needs a CI run before merging label Jul 17, 2025
@jrosain
Copy link
Member

jrosain commented Jul 17, 2025

Your branch seems to be in a strange state with the merge commit and everything.

@jrosain jrosain changed the title Update CONTRIBUTING.md Update PR instructions in CONTRIBUTING.md Jul 19, 2025
@jrosain jrosain removed the needs:ci Needs a CI run before merging label Jul 19, 2025
@jrosain jrosain added the needs:progress The PR needs to be updated w.r.t. the discussions label Aug 25, 2025
Update Contributing.md and .gitignore

Remarks
@jcailler jcailler removed the needs:progress The PR needs to be updated w.r.t. the discussions label Aug 29, 2025
```

where `remote` is the name of the remote fetching `git@github.com:GoelandProver/Goeland.git` (which can be found using `git remote -v`)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like a transition is missing between these two paragraphs. Maybe something like: "If you want to open a pull request, you should roughly follow these steps:"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind:documentation Enhance or add documentation part:infrastructure The PR is on non-goéland code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants