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.

Sign up for GitHub

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

Jump to bottom

#211: Release notes entry enforcement #212

Merged
merged 32 commits into from
Jun 24, 2024
Merged

#211: Release notes entry enforcement #212

merged 32 commits into from
Jun 24, 2024

Conversation

benedeki
Copy link
Contributor

@benedeki benedeki commented Jun 13, 2024 •
edited
Loading

  • changed the label to skip issue for release notes from default to no RN
  • added action to verify a PR has Release note comment, unless marked with no RN label
  • added action to copy Release note comment from PR to linked issues upon merge to master
  • added an action to check test filenames

Closes #211

@benedeki
#211: Release notes entry enforcement
e77987e 
* changed the label to skip issue for release notes from default to `no RN`
* added action to verify a PR has Release note comment, unless marked with `no RN` label
* added action to copy Release note comment from PR to linked issues upon merge to master
@benedeki benedeki self-assigned this Jun 13, 2024
@benedeki benedeki added no RN No release notes required and removed no RN No release notes required labels Jun 13, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 13, 2024 •
edited
Loading

JaCoCo model module code coverage report - scala 2.13.11

Overall Project 60.74% 🍏

There is no coverage information present for the Files changed

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 13, 2024 •
edited
Loading

JaCoCo agent module code coverage report - scala 2.13.11

Overall Project 86.35% 🍏

There is no coverage information present for the Files changed

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 13, 2024 •
edited
Loading

JaCoCo server module code coverage report - scala 2.13.11

Overall Project 64.04% 🍏

There is no coverage information present for the Files changed

@benedeki benedeki added the work in progress Work on this item is not yet finished (mainly intended for PRs) label Jun 13, 2024
@benedeki
* added on-issue-comment trigger
6e0afae 
* improved filtering (hopefully)
@benedeki benedeki added the no RN No release notes required label Jun 14, 2024
@benedeki benedeki removed the no RN No release notes required label Jun 14, 2024
@benedeki benedeki added no RN No release notes required and removed no RN No release notes required work in progress Work on this item is not yet finished (mainly intended for PRs) labels Jun 14, 2024
@benedeki benedeki removed the work in progress Work on this item is not yet finished (mainly intended for PRs) label Jun 19, 2024
@miroslavpojer miroslavpojer added no RN No release notes required and removed no RN No release notes required labels Jun 20, 2024
benedeki and others added 2 commits June 20, 2024 08:25
@benedeki @miroslavpojer
Apply suggestions from code review
50be6b2 
Co-authored-by: miroslavpojer <miroslav.pojer@absa.africa>
@benedeki
* removed demo code
d40d31e
miroslavpojer
miroslavpojer previously approved these changes Jun 20, 2024
View reviewed changes
lsulak
lsulak reviewed Jun 20, 2024
View reviewed changes
Copy link
Collaborator

@lsulak lsulak left a comment

Choose a reason for hiding this comment

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

My review finished for now, added some comments

lsulak
lsulak reviewed Jun 20, 2024
View reviewed changes
.github/workflows/test_filenames_check.yml
@@ -36,5 +36,6 @@ jobs:
name-patterns: '*UnitTests.*,*IntegrationTests.*'
paths: '**/src/test/scala/**'
report-format: 'console'
excludes: 'server/src/test/scala/za/co/absa/atum/server/api/TestData.scala,server/src/test/scala/za/co/absa/atum/server/api/TestTransactorProvider.scala,server/src/test/scala/za/co/absa/atum/server/ConfigProviderTest.scala'
Copy link
Collaborator

Choose a reason for hiding this comment

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

is it possiblE to add multi-line exclusions instead of 1 line? Also, is it possible to regexp it? Or at least use wildcard(s)?

Copy link
Collaborator

@miroslavpojer miroslavpojer Jun 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

For multi-line exclusion, see and use filename-inspector branch feature/fix-to-use-correct-syntax in your yaml.

  • See updated README.md for an example.

Regex

  • paths support glob patterns (supports **)
  • name-patterns and excludes support fnmatch pattern (do not support **)

Feel free to help with PR review on named branch and its PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think wildcards are possible. RegExpt no, at least not yet now (it hasn't been released yet even).
I actually thought to move the classes into own directories, to make the exclusions simpler. But in its own PR.

Copy link
Collaborator

@miroslavpojer miroslavpojer Jun 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Add wildcards, here is my testing example:

- name: Test filename-inspector check
  id: test-file-suffix
  uses: AbsaOSS/filename-inspector@feature/fix-to-use-correct-syntax
  with:
    name-patterns: |
      *GUI*,
      *Unit*
    paths: |
      **/src/test/java/**,
      **/src/test/scala/**
    excludes: |
      *1UnitTest.*,
      *thServiceTest.*
    report-format: 'csv'
    verbose-logging: 'true'
    fail-on-violation: 'true'

See link.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The multiline example doesn't seem to work. 😠 Reverting to single line, that worked.
(Tried all kind of formats)

lsulak
lsulak approved these changes Jun 24, 2024
View reviewed changes
Copy link
Collaborator

@lsulak lsulak left a comment

Choose a reason for hiding this comment

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

approved!

@benedeki benedeki merged commit f5ac426 into master Jun 24, 2024
10 checks passed
@benedeki benedeki deleted the feature/211-release-notes-entry-enforcement branch June 24, 2024 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Zejnilovic Zejnilovic Zejnilovic left review comments

@miroslavpojer miroslavpojer miroslavpojer left review comments

@lsulak lsulak lsulak approved these changes

@TebaleloS TebaleloS Awaiting requested review from TebaleloS TebaleloS is a code owner

@dk1844 dk1844 Awaiting requested review from dk1844 dk1844 is a code owner

@salamonpavel salamonpavel Awaiting requested review from salamonpavel salamonpavel is a code owner

Assignees

@benedeki benedeki

Labels
no RN No release notes required
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Release notes entry enforcement
4 participants
@benedeki @lsulak @Zejnilovic @miroslavpojer