Skip to content
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

feat: Add author validator #710

Merged

Conversation

robinmayerhofer
Copy link
Contributor

Goal

Add an author validator that behaves exactly like the author filter.

How

I created lib/validators/author.js, which is basically the same as lib/filters/author.js. I saw no convention on how to do an object that can be a filter and validator at the same time.

I also created lib/validators/options_processor/options/team.js, which is pretty much what lib/filters/options_processor/options/team.js is. However, there is a subtle difference when implementing it: the method signature for process contains an extra argument for filters, thus, we cannot reuse the TeamProcessor.

@shine2lay shine2lay merged commit 64d4afd into mergeability:master Apr 25, 2023
2 checks passed
@robinmayerhofer robinmayerhofer deleted the feat-author-validator branch April 25, 2023 16:36
@robinmayerhofer
Copy link
Contributor Author

Hi @shine2lay - thanks for the very fast review & merge!

I've checked the release job, and it seems like it ran but did not release anything.

[4:37:22 PM] [semantic-release] › ℹ  Found git tag v2.17.2 associated with version 2.17.2 on branch master
[31](https://github.com/mergeability/mergeable/actions/runs/4799976152/jobs/8540311837#step:5:32)
[4:37:22 PM] [semantic-release] › ℹ  Found 1 commits since last release
[32](https://github.com/mergeability/mergeable/actions/runs/4799976152/jobs/8540311837#step:5:33)
[4:37:22 PM] [semantic-release] › ℹ  Start step "analyzeCommits" of plugin "@semantic-release/commit-analyzer"
[33](https://github.com/mergeability/mergeable/actions/runs/4799976152/jobs/8540311837#step:5:34)
[4:37:22 PM] [semantic-release] [@semantic-release/commit-analyzer] › ℹ  Analyzing commit: Add author validator (#710)
[34](https://github.com/mergeability/mergeable/actions/runs/4799976152/jobs/8540311837#step:5:35)
[4:37:22 PM] [semantic-release] [@semantic-release/commit-analyzer] › ℹ  The commit should not trigger a release
[35](https://github.com/mergeability/mergeable/actions/runs/4799976152/jobs/8540311837#step:5:36)
[4:37:22 PM] [semantic-release] [@semantic-release/commit-analyzer] › ℹ  Analysis of 1 commits complete: no release
[36](https://github.com/mergeability/mergeable/actions/runs/4799976152/jobs/8540311837#step:5:37)
[4:37:22 PM] [semantic-release] › ✔  Completed step "analyzeCommits" of plugin "@semantic-release/commit-analyzer"
[37](https://github.com/mergeability/mergeable/actions/runs/4799976152/jobs/8540311837#step:5:38)
[4:37:22 PM] [semantic-release] › ℹ  There are no relevant changes, so no new version is released.

Is this because the commit message on master misses the feat: prefix? How could we get a release out for this?

@shine2lay
Copy link
Member

@RM98 tbh, i am not sure myself, it seems it stops since a few PRs back. would you mind taking a look? worse case, scenario, i think I should be able to do a manual release

@robinmayerhofer
Copy link
Contributor Author

Hey! I checked the latest release jobs that ran - and I believe that the issue for this PR is the commit message. It needs the feat prefix.

Regarding the previous PR (link to the release job run):

The previous release worked (npm publishing & creating the release in the GitHub repo), but adding the comment to the PR did not work because of some rate-limiting issues (see here).
Re-running did not add the comment to the PR, because the NPM package/GitHub release already exists.

I don't see this as a big problem - and the fix would need to happen on the side of the semantic-release plugin I guess (support checking whether the comment was posted or not).

=> I would neglect this & prefer triggering a manual release (or merging something with an appropriate commit message that then triggers a release)

@shine2lay
Copy link
Member

i've tagged and released + published

@robinmayerhofer
Copy link
Contributor Author

TYVM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants