-
Notifications
You must be signed in to change notification settings - Fork 6
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
Adding PR submission section to contributors guide #43
Adding PR submission section to contributors guide #43
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, although it is not mandatory for a PR to have an associated issue referenced
does appear to conflict with the PR workflow that's trying to be added to ensure every PR has a linked issue?
e.g. the one that's failing on this PR 😂
|
||
The Kuadrant project owners and maintainers strive to review and/or respond to all newly submitted PRs in a timely manner, however, if you're finding it difficult to get someone to review your PR, please reach out directly on [Slack][SlackChannelURL] or [mail][MailingList] | ||
|
||
Finally, it is recommended that you squash your changes into a single commit where possible. If this is not feasible please ensure that your commits are representing a logical piece of work that can be reviewed independently within the PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In some repos, we also require signed commits. E.g. https://docs.kuadrant.io/authorino/docs/contributing/#sign-your-commits
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @guicassolato I'll add something based on what you've referenced in the Developer guide. I think there's a much larger piece of work to be done here with respect to having a consistent message across the various Kuadrant repos calling back to a single developer guide with perhaps a few adjustments in each repo. I think the Authorino developer guide should form the basis for the others given it's depth and structure. I'll take an action to try progress this effort in the new year.
For now though I'll add the signed commits section and look to get this merged as a first pass
7e991d9
to
22f52a2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: guicassolato, philbrookes, pmccarthy The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Hmmm..I thought the purpose of that job was to ensure the PR and/or issue is showing on our project board? Rather than ensuring that there's an associated issue? Perhaps I'm wrong on that? |
You are correct, @pmccarthy . |
oh, interesting, so if there is no linked issue, this job should not fail? I feel like it does currently. |
We're still investigating why the job is failing, but I guess it has nothing to do with having or not having an issue linked to the PR. The PR itself is a kind of GitHub issue. AFAIK the job is only to set the Kuadrant project on all kinds of issues (actual issues or PRs.) |
What
Adding new PR submission section to the contributors guide. This section is purposely high level as we'll most likely want to add some component specific guidelines in each repository