Merge pull request #4037 from element-hq/feature/bma/pullRequestTemplate

Update pull request template and CI automation
This commit is contained in:
Benoit Marty
2024-12-30 09:34:19 +01:00
committed by GitHub
4 changed files with 5 additions and 79 deletions

View File

@@ -44,11 +44,10 @@ Uncomment this markdown table below and edit the last line `|||`:
<!-- Depending on the Pull Request content, it can be acceptable if some of the following checkboxes stay unchecked. -->
- [ ] Changes have been tested on an Android device or Android emulator with API 23
- [ ] Changes have been tested on an Android device or Android emulator with API 24
- [ ] UI change has been tested on both light and dark themes
- [ ] Accessibility has been taken into account. See https://github.com/element-hq/element-x-android/blob/develop/CONTRIBUTING.md#accessibility
- [ ] Pull request is based on the develop branch
- [ ] Pull request title will be used in the release note, it clearly define what will change for the user
- [ ] Pull request includes screenshots or videos if containing UI changes
- [ ] Pull request includes a [sign off](https://matrix-org.github.io/synapse/latest/development/contributing_guide.html#sign-off)
- [ ] You've made a self review of your PR

View File

@@ -24,7 +24,6 @@ Here are the checks that Danger does so far:
- Big PR got a warning to recommend to split
- PR contains a correct title and a label to categorize the release note
- PR does not modify frozen classes
- PR contains a Sign-Off, with exception for Element employee contributors
- PR with change on layout should include screenshot in the description (TODO Not supported yet!)
- PR which adds png file warn about the usage of vector drawables
- non draft PR should have a reviewer

View File

@@ -173,7 +173,7 @@ But comment in PR from the community are always appreciated!
4. Fork consideration. Will configuration of forks be easy? Some documentation may help in some cases.
5. We are building long term products. "Quick and dirty" code must be avoided.
6. The PR includes new tests for the added code, updated test for the existing code
7. All PRs from external contributors **MUST** include a sign-off. It's in the checklist, and sometimes it's checked by the submitter, but there is actually no sign-off. In this case, ask nicely for a sign-off and request changes (do not approve the PR, even if everything else is fine).
7. All commit authors must have signed the CLA. Please open https://cla-assistant.io/element-hq/element-x-android to agree to the CLA.
### Rules

View File

@@ -48,72 +48,6 @@ frozenClasses.forEach(frozen => {
}
)
// Check for a sign-off
const signOff = "Signed-off-by:"
// Please add new names following the alphabetical order.
const allowList = [
"aringenbach",
"BillCarsonFr",
"bmarty",
"csmith",
"dependabot[bot]",
"Florian14",
"ganfra",
"github-actions[bot]",
"jmartinesp",
"jonnyandrew",
"julioromano",
"kittykat",
"langleyd",
"MadLittleMods",
"manuroe",
"renovate[bot]",
"stefanceriu",
"yostyle",
]
function signoff_needed(reason) {
message("Sign-off required, " + reason)
const hasPRBodySignOff = pr.body.includes(signOff)
const hasCommitSignOff = danger.git.commits.every(commit => commit.message.includes(signOff))
if (!hasPRBodySignOff && !hasCommitSignOff) {
fail("Please add a sign-off to either the PR description or to the commits themselves. See instructions [here](https://matrix-org.github.io/synapse/latest/development/contributing_guide.html#sign-off).")
}
}
function signoff_unneeded(reason) {
message("Sign-off not required, " + reason)
}
// Somewhat awkward phrasing, dangerfile is not in an async context.
if (allowList.includes(user)) {
signoff_unneeded("allow-list")
} else {
// github.api.rest.orgs.checkMembershipForUser({
// org: "element-hq",
// username: user,
// }).then((result) => {
github.api.rest.teams.getMembershipForUserInOrg({
org: "element-hq",
team_slug: "vector-core",
username: user,
}).then((result) => {
if (result.status == 204 || result.status == 200) {
signoff_unneeded("team-member")
}
else {
signoff_needed("not-team-member")
}
}).catch((error) => {
if (error.response.status == 404) {
signoff_needed("not-team-member");
} else {
console.log(error); signoff_needed("error")
}
})
}
const previewAnnotations = [
'androidx.compose.ui.tooling.preview.Preview',
'io.element.android.libraries.designsystem.preview.PreviewWithLargeHeight',
@@ -170,17 +104,11 @@ if (hasPngs) {
// Check that translations have not been modified by developers
const translationAllowList = [
"RiotTranslateBot",
"github-actions[bot]",
"ElementBot",
]
if (!translationAllowList.includes(user)) {
if (editedFiles.some(file => file.endsWith("strings.xml") && !file.endsWith("values/strings.xml"))) {
fail("Some translation files have been edited. Only user `RiotTranslateBot` (i.e. translations coming from Weblate) or `github-actions[bot]` (i.e. translations coming from automation) are allowed to do that.\nPlease read more about translations management [in the doc](https://github.com/element-hq/element-android/blob/develop/CONTRIBUTING.md#internationalisation).")
}
// Check that new strings are not added to `values/strings.xml`
if (editedFiles.some(file => file.endsWith("ui-strings/src/main/res/values/strings.xml"))) {
fail("`ui-strings/src/main/res/values/strings.xml` has been edited. This file will be overridden in the next strings synchronisation. Please add new strings in the file `values/strings_eax.xml` instead.")
if (editedFiles.some(file => file.endsWith("translations.xml"))) {
fail("Some translation files have been edited. Only user `ElementBot` (i.e. translations coming from Localazy) is allowed to do that.\nPlease read more about translations management [in the doc](https://github.com/element-hq/element-x-android/blob/develop/CONTRIBUTING.md#strings).")
}
}