Configure Danger, enforce labels #1761
Conversation
…re-changelog-updates-on-every-pr
vegaro
changed the title
|
That's great feedback @NachoSoto and I agree with you. I think the Danger plugin also checks if the description of the PR has the jira ticket, let me confirm. Do you think that would be better? |
|
ok so I retested it @NachoSoto and it looks like it also looks for the JIRA issue in the PR body and the PR title. I think having it in the body is good right? If we have external contributors we can edit the PR body or the PR title ourselves too. What do you think? |
That would be great! |
|
I removed the changelog check stuff because we want to automate it in and not require it in the PRs :) |
Dangerfile
Outdated
| search_commits: false, | ||
| fail_on_warning: false, | ||
| report_missing: true, | ||
| skippable: true # skippable by adding [no-jira] to PR title or body |
There was a problem hiding this comment.
🧡 like the skippable, but maybe also enforce a skip reason? Like if you use the [no-jira] we should have a description that makes it obvious why we skipped? like require skip jira reason:
Thoughts?
There was a problem hiding this comment.
I like it. The thing is that this is a plugin and it doesn't have the option. I would also like to change the warning message. Maybe I fork it?
There was a problem hiding this comment.
Yeah, it looks abandoned 😿 I think your best bet is a fork. I looked at the other forks, and nothing sticks out as one we should use.
There was a problem hiding this comment.
Yes... the goal of this sprint is to work on release trains so I might merge it like this and work on the fork in the future. We are going to still use Danger for the release trains (to check if PRs are label correctly) so this PR is still useful.
Do you think we can still use this plugin until we fork it? Or should we just not use it at all
vegaro
changed the title
…s-set-up-danger-to-ensure-changelog-updates-on-every-pr # Conflicts: # Gemfile.lock
|
I removed the danger-jira stuff since we would like to do some stuff the plugin doesn't support:
I think it's better to leave it for another sprint and focus on releases this sprint |
|
I created a new bot RevenueCat-Danger-Bot because it is recommended in https://danger.systems/guides/getting_started.html#oss-projects that the bots don't have access to the repositories and RCGitBot has access. I am not sure what the security concern is. I guess the problem is that the token could be leaked in CI. But in that case the RCGitBot token we have right now can also be leaked right? |
vegaro
changed the title
Dangerfile
Outdated
| no_supported_label = supported_labels_in_pr.empty? | ||
| if no_supported_label | ||
| fail("Label the PR using one of the change type labels: #{supported_types}") | ||
| end |
There was a problem hiding this comment.
I haven't used Danger before so I don't know but just wondering if this could be introduced in a function and then call that function, in case we add other validations in the future.
| @@ -534,3 +554,6 @@ workflows: | |||
| - docs-deploy: | |||
| xcode_version: '14.0.0' | |||
| <<: *release-tags | |||
| danger: | |||
| jobs: | |||
| - danger | |||
There was a problem hiding this comment.
Do we need to specify in what branches we need to run this? I guess we want every branch that has a PR associated? (Though not sure how to do that in circleci)
There was a problem hiding this comment.
With the settings we have in CircleCI for purchases-ios it will work like that and this will run in every branch with a PR
There was a problem hiding this comment.
Oh interesting... I don't think that's the case for all our repos, which might lead to inconsistencies. But it should be ok in this repo then 👍
vegaro
deleted the
CSDK-127-purchases-ios-set-up-danger-to-ensure-changelog-updates-on-every-pr
branch
This is how the check looks like:
This PR initially added Jira link checks, but the lack of customization on the danger-jira plugin made me decide to drop the danger check. We should probably write our own/fork that plugin and add more options.
CSDK-127