-
-
Notifications
You must be signed in to change notification settings - Fork 121
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: outputs the error messages #194
Changes from 2 commits
958060d
4f18557
a0adbcd
278a1ec
7c2e077
8f50de9
1800f47
eae7697
44f0be2
cb5e1ae
70b5725
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,13 @@ | ||
const core = require('@actions/core'); | ||
const conventionalCommitsConfig = require('conventional-changelog-conventionalcommits'); | ||
const conventionalCommitTypes = require('conventional-commit-types'); | ||
const parser = require('conventional-commits-parser').sync; | ||
const formatMessage = require('./formatMessage'); | ||
|
||
const defaultTypes = Object.keys(conventionalCommitTypes.types); | ||
|
||
let errorMessage; | ||
|
||
module.exports = async function validatePrTitle( | ||
prTitle, | ||
{ | ||
|
@@ -52,30 +55,28 @@ module.exports = async function validatePrTitle( | |
} | ||
|
||
if (!result.type) { | ||
throw new Error( | ||
`No release type found in pull request title "${prTitle}". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/\n\n${printAvailableTypes()}` | ||
); | ||
errorMessage = `No release type found in pull request title "${prTitle}". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/\n\n${printAvailableTypes()}`; | ||
throw new Error(errorMessage); | ||
} | ||
|
||
if (!result.subject) { | ||
throw new Error(`No subject found in pull request title "${prTitle}".`); | ||
errorMessage = `No subject found in pull request title "${prTitle}".`; | ||
throw new Error(errorMessage); | ||
} | ||
|
||
if (!types.includes(result.type)) { | ||
throw new Error( | ||
`Unknown release type "${ | ||
result.type | ||
}" found in pull request title "${prTitle}". \n\n${printAvailableTypes()}` | ||
); | ||
errorMessage = `Unknown release type "${ | ||
result.type | ||
}" found in pull request title "${prTitle}". \n\n${printAvailableTypes()}`; | ||
throw new Error(errorMessage); | ||
} | ||
|
||
if (requireScope && !result.scope) { | ||
let msg = `No scope found in pull request title "${prTitle}".`; | ||
errorMessage = `No scope found in pull request title "${prTitle}".`; | ||
if (scopes) { | ||
msg += ` Use one of the available scopes: ${scopes.join(', ')}.`; | ||
errorMessage += ` Use one of the available scopes: ${scopes.join(', ')}.`; | ||
} | ||
|
||
throw new Error(msg); | ||
throw new Error(errorMessage); | ||
} | ||
|
||
const givenScopes = result.scope | ||
|
@@ -84,26 +85,24 @@ module.exports = async function validatePrTitle( | |
|
||
const unknownScopes = givenScopes ? givenScopes.filter(isUnknownScope) : []; | ||
if (scopes && unknownScopes.length > 0) { | ||
throw new Error( | ||
`Unknown ${ | ||
unknownScopes.length > 1 ? 'scopes' : 'scope' | ||
} "${unknownScopes.join( | ||
',' | ||
)}" found in pull request title "${prTitle}". Use one of the available scopes: ${scopes.join( | ||
', ' | ||
)}.` | ||
); | ||
errorMessage = `Unknown ${ | ||
unknownScopes.length > 1 ? 'scopes' : 'scope' | ||
} "${unknownScopes.join( | ||
',' | ||
)}" found in pull request title "${prTitle}". Use one of the available scopes: ${scopes.join( | ||
', ' | ||
)}.`; | ||
throw new Error(errorMessage); | ||
} | ||
|
||
const disallowedScopes = givenScopes | ||
? givenScopes.filter(isDisallowedScope) | ||
: []; | ||
if (disallowScopes && disallowedScopes.length > 0) { | ||
throw new Error( | ||
`Disallowed ${ | ||
disallowedScopes.length === 1 ? 'scope was' : 'scopes were' | ||
} found: ${disallowScopes.join(', ')}` | ||
); | ||
errorMessage = `Disallowed ${ | ||
disallowedScopes.length === 1 ? 'scope was' : 'scopes were' | ||
} found: ${disallowScopes.join(', ')}`; | ||
throw new Error(errorMessage); | ||
} | ||
|
||
function throwSubjectPatternError(message) { | ||
|
@@ -114,6 +113,8 @@ module.exports = async function validatePrTitle( | |
}); | ||
} | ||
|
||
errorMessage = message; | ||
|
||
throw new Error(message); | ||
} | ||
|
||
|
@@ -133,4 +134,6 @@ module.exports = async function validatePrTitle( | |
); | ||
} | ||
} | ||
|
||
core.setOutput('error_message', errorMessage); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you have some reasoning on why you picked There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, I only based my choice on the Conventional Changelog Action, which uses snake_case for the outputs. https://github.com/TriPSs/conventional-changelog-action I can look into some more famous examples, but the final decision is yours xD what do you prefer? camelCase? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I recommend using as it is in docs, upper case and underscore https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea, base it on docs seems like the best option imo There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the suggestion @derberg! I just checked the docs and it seems like they're a bit inconsistent with the casing:
It seems like there's unfortunately not really a standard here. However lower case snake case seems to be the most popular one and also what convitional-changelog-action is using, so I'd vote to go for that. |
||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we throw here, the
core.setOutput
statement below is never called. You could introduce a new function that makes sure we always do both:… and use that for all validation errors here.
That way you also don't need the variable introduced above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, good catch, a silly mistake on my part. Will refactor that later.