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

Add changelog lint cmd #119

Merged
merged 6 commits into from
Nov 3, 2022
Merged

Conversation

lucian-ioan
Copy link
Collaborator

@lucian-ioan lucian-ioan commented Oct 28, 2022

Closes #45

Cherry-pick of a7d4406 is required. PR #116 should be merged first.

@elasticmachine
Copy link
Collaborator

elasticmachine commented Oct 28, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-11-03T09:24:47.571+0000

  • Duration: 3 min 50 sec

Test stats 🧪

Test Results
Failed 0
Passed 85
Skipped 0
Total 85

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@endorama
Copy link
Member

I like the approach, I added a commit that extracts the validators to separate functions, as we need to add tests for them to ensure long term maintainability.

Once that is done this will be good to merge.

@endorama endorama modified the milestone: v0.4.0 Oct 31, 2022
"github.com/stretchr/testify/require"
)

func TestValidators(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Break this down per validator, as it is now would fail for any validator and that may be confusing to future contributors.

main.go Outdated
@@ -25,6 +25,7 @@ func main() {
rootCmd.AddCommand(cmd.PrHasFragmentCommand(appFs))
rootCmd.AddCommand(cmd.RenderCmd(appFs))
rootCmd.AddCommand(cmd.VersionCmd())
rootCmd.AddCommand(cmd.ChangelogLintCmd(appFs))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add this in alphabetical order

@lucian-ioan lucian-ioan merged commit 6e99924 into elastic:main Nov 3, 2022
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.

Consolidated changelog linter
3 participants