|
| 1 | +# Contribution guidelines |
| 2 | + |
| 3 | +## If you have found a bug or would like to see a new feature |
| 4 | + |
| 5 | +Please reach us by creating a [new issue]. |
| 6 | + |
| 7 | +Your bug report should include a proper description and steps to reproduce: |
| 8 | +- attach the LLVM BC or SPV file you are trying to translate and the command you |
| 9 | + launch |
| 10 | +- any backtrace in case of crashes would be helpful |
| 11 | +- please describe what goes wrong or what is unexpected during translation |
| 12 | + |
| 13 | +For feature requests, please describe the feature you would like to see |
| 14 | +implemented in the translator. |
| 15 | + |
| 16 | +[new issue]: https://github.com/KhronosGroup/SPIRV-LLVM-Translator/issues/new |
| 17 | + |
| 18 | +## If you would like to contribute your change |
| 19 | + |
| 20 | +Please open a [pull request]. If you are not sure whether your changes are |
| 21 | +correct, you can either mark it as [draft] or create an issue to discuss the |
| 22 | +problem and possible ways to fix it prior to publishing a PR. |
| 23 | + |
| 24 | +It is okay to have several commits in the PR, but each of them should be |
| 25 | +buildable and tests should pass. Maintainers can squash several commits |
| 26 | +into a single one for you during merge, but if you would like to see several |
| 27 | +commits in the git history, please let us know in PR description/comments so |
| 28 | +maintainers will rebase your PR instead of squashing it. |
| 29 | + |
| 30 | +Each functional change (new feature or bug fix) must be supplied with |
| 31 | +corresponding tests. See [#testing-guidelines] for more information about |
| 32 | +testing. NFC (non-functional change) PRs can be accepted without new tests. |
| 33 | + |
| 34 | +Code changes should follow coding standards, which are inherited from [LLVM |
| 35 | +Coding Standards]. Compliance of your code is checked automatically using |
| 36 | +Travis CI. See [clang-format] and [clang-tidy] configs for more details about |
| 37 | +coding standards. |
| 38 | + |
| 39 | +### Conditions to merge a PR |
| 40 | + |
| 41 | +In order to get your PR merged, the following conditions must be met: |
| 42 | +- If you are a first-time contributor, you have to sign the |
| 43 | + [Contributor License Agreement]. Corresponding link and instructions will be |
| 44 | + automatically posted into your PR. |
| 45 | +- [Travis CI testing] jobs must pass on your PR: this includes functional |
| 46 | + testing and checking for complying with coding standards. |
| 47 | +- You need to get approval from at least one contributor with merge rights. |
| 48 | + |
| 49 | +As a contributor, you should expect that even an approved PR might still be left |
| 50 | +open for a few days: this is needed, because the translator is being developed |
| 51 | +by different vendors and individuals and we need to ensure that each interested |
| 52 | +party is able to react to new changes and provide feedback. |
| 53 | + |
| 54 | +Information below is a guideline for repo maintainers and can be used by |
| 55 | +contributors to get some expectations about how long a PR has to be open before |
| 56 | +it can be merged: |
| 57 | +- For any significant change/redesign, the PR must be open for at least 5 |
| 58 | + working days, so everyone interested can step in to provide feedback, discuss |
| 59 | + direction and help to find bugs. |
| 60 | + - Ideally, there should be approvals from different vendors/individuals to get |
| 61 | + it merged, particularly for larger changes. |
| 62 | +- For regular changes/bug fixes, the PR must be open for at least 2-3 working |
| 63 | + days, so everyone interested can step in for review and provide feedback. |
| 64 | + - If the change is vendor-specific (bug fix in vendor extension implementation |
| 65 | + or new vendor-specific extension support), then it is okay to merge PR |
| 66 | + sooner. |
| 67 | + - If the change affects or might affect several interested parties, the PR |
| 68 | + must be left open for 2-3 working days and it would be good to see feedback |
| 69 | + from different vendors/inviduals before merging. |
| 70 | +- Tiny NFC changes or trivial build fixes (due to LLVM API changes) can be |
| 71 | + submitted as soon as testing is finished and PR approved - no need to wait for |
| 72 | + too long. |
| 73 | +- In general, just use common sense to wait long enough to get feedback from |
| 74 | + everyone who might be interested in the PR and don't hesitate to explicitly |
| 75 | + mention individuals who might be interested in reviewing the PR. |
| 76 | + |
| 77 | +[pull request]: https://github.com/KhronosGroup/SPIRV-LLVM-Translator/pulls |
| 78 | +[draft]: https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/about-pull-requests#draft-pull-requests |
| 79 | +[LLVM Coding Standards]: https://llvm.org/docs/CodingStandards.html |
| 80 | +[clang-format]: [.clang-format] |
| 81 | +[clang-tidy]: [.clang-tidy] |
| 82 | +[Contributor License Agreement]: https://cla-assistant.io/KhronosGroup/SPIRV-LLVM-Translator |
| 83 | +[Travis CI testing]: https://travis-ci.org/KhronosGroup/SPIRV-LLVM-Translator |
0 commit comments