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

Fix #192, Adds check commit message job in format check #470

Merged
merged 1 commit into from
May 19, 2022

Conversation

chillfig
Copy link
Contributor

@chillfig chillfig commented Apr 27, 2022

Checklist (Please check before submitting)

Describe the contribution
A clear and concise description of what the contribution is.

Testing performed
Tested on fork: https://github.com/chillfig/cFS/runs/6427976618?check_suite_focus=true

Expected behavior changes
Commit messages not following any of the below conventions will fail the format-check.yml

  • Fix #XYZ, DESCRIPTION
  • HotFix #XYZ, DESCRIPTION
  • Partial #XYZ, DESCRIPTION
  • Merge pull request #XYZ DESCRIPTION
  • IC: DESCRIPTION

System(s) tested on
Ubuntu 18.04

Additional context
The regex for DESCRIPTION is defined as a string of 1 or more alphanumeric characters.
The regex for XYZ is defined as a string of 1 or more numeric characters.

Code contributions
The cFS repository is provided to bundle the cFS Framework. It is utilized for bundling submodules, continuous integration testing, and version management and does not contain any software. Code contributions should be directed to the appropriate submodule.

Contributor Info - All information REQUIRED for consideration of pull request
Justin Figueroa, ASRC Federal

@chillfig chillfig self-assigned this Apr 27, 2022
@chillfig chillfig added continuous-integration CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Apr 27, 2022
@astrogeco
Copy link
Contributor

astrogeco commented May 4, 2022

CCB:2022-05-04, APPROVED with CHANGES

  • Spin off into its own reusable workflow (triggered on workflow_call)
  • Consider replacing "partial" with "part"? Do we have a "better" short word for that?

@astrogeco
Copy link
Contributor

astrogeco commented May 4, 2022

@skliper now that I think about it, for maintenance maybe we should still combine it in the "caller" workflow but restrict it to run on pull_requests? If we separate them we'll have to create and maintain one of these for each single repo.

@skliper
Copy link
Contributor

skliper commented May 4, 2022

@astrogeco are you able to separate it into it's own job that only runs on pull requests even when triggered from a workflow call? That would be enough to separate it from the code format check (different job) and avoid nuisance failures (only on pull request) for me. Although if there's plans to add more checking to this workflow a name update may help clarify things (not just checking code whitespace formatting anymore).

@chillfig
Copy link
Contributor Author

chillfig commented May 4, 2022

@astrogeco are you able to separate it into it's own job that only runs on pull requests even when triggered from a workflow call? That would be enough to separate it from the code format check (different job) and avoid nuisance failures (only on pull request) for me. Although if there's plans to add more checking to this workflow a name update may help clarify things (not just checking code whitespace formatting anymore).

Interesting. Yes, the "Check commit message" step can be changed into its own job in format-check.yml with the addition of if: github.event_name == 'pull_request'. According to https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idif, that addition should keep the check commit message job to only running on pull requests.

@chillfig chillfig force-pushed the commit_msg_check branch from 600b4ae to c05b5e3 Compare May 5, 2022 14:37
Comment on lines 80 to 81
pattern: '^((Fix|HotFix|Partial)\s\#[0-9]+,\s[a-zA-Z0-9]+|Merge\spull\srequest\s\#[0-9]+\s[a-zA-Z0-9]+|IC:\s[a-zA-Z0-9]+)'
error: 'You need at least one "Fix #<issue number>, <short description>" line. "HotFix" and "Partial" are also acceptable.'
Copy link
Contributor

Choose a reason for hiding this comment

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

How about "Part" vs "Partial"?

@chillfig chillfig force-pushed the commit_msg_check branch from c05b5e3 to e8d78e8 Compare May 5, 2022 22:40
Adds pattern that accepts commit msgs with "Fix [OR] HotFix [OR] Partial #XYZ <description>",
Adds pattern that accepts commit msgs with "Merge pull request #XYZ <description>",
Adds pattern that accepts commit msgs with "IC: <description>"
Adds conditional for commit msg job to run only on pull-requests
Splits msg check for PR title and each of its commits
Adds contextual comments to each step
Adds implicit default flags to each step for readability
@chillfig chillfig force-pushed the commit_msg_check branch from e8d78e8 to efbb729 Compare May 13, 2022 17:23
@chillfig chillfig added CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels May 16, 2022
@astrogeco
Copy link
Contributor

CCB:2022-05-18 APPROVED

@astrogeco astrogeco added CCB:Approved Indicates code review and approval by community CCB CCB:FastTrack labels May 18, 2022
@astrogeco astrogeco merged commit 7bda14a into nasa:main May 19, 2022
@chillfig chillfig deleted the commit_msg_check branch June 3, 2022 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CCB:Approved Indicates code review and approval by community CCB CCB:FastTrack continuous-integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check that commit messages adhere to standard using the continuous integration workflow
3 participants