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

feat: outputs the error messages #194

Merged
merged 11 commits into from Sep 26, 2022
Merged

feat: outputs the error messages #194

Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
958060d
feat: outputs the error messages
juninholiveira Jul 20, 2022
4f18557
style: run eslint src --fix
juninholiveira Jul 20, 2022
a0adbcd
fix: setOutput never getting called
juninholiveira Jul 20, 2022
278a1ec
refactor: remove unused variable
juninholiveira Jul 20, 2022
7c2e077
refactor: remove unused call
juninholiveira Jul 20, 2022
8f50de9
refactor: change output name to official docs style
juninholiveira Jul 20, 2022
1800f47
feat: create new workflow to test output messages
juninholiveira Jul 20, 2022
eae7697
docs: add information about outputs on readme.md
juninholiveira Jul 21, 2022
44f0be2
Lowercase error_message
amannn Jul 25, 2022
cb5e1ae
Shorten docs
amannn Jul 25, 2022
70b5725
Improved example
amannn Sep 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
57 changes: 30 additions & 27 deletions src/validatePrTitle.js
View file
Open in desktop
@@ -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,
{
Expand Down Expand Up @@ -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);
Copy link
Owner

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:

function raiseError(message) {
  core.setOutput('errorMessage', message);
  throw new Error(message);
}

… and use that for all validation errors here.

That way you also don't need the variable introduced above.

Copy link
Contributor Author

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.

}

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
Expand All @@ -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) {
Expand All @@ -114,6 +113,8 @@ module.exports = async function validatePrTitle(
});
}

errorMessage = message;

throw new Error(message);
}

Expand All @@ -133,4 +134,6 @@ module.exports = async function validatePrTitle(
);
}
}

core.setOutput('error_message', errorMessage);
Copy link
Owner

Choose a reason for hiding this comment

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

Do you have some reasoning on why you picked error_message? Generally we use camelCase, so in doubt I'd go for that here too. It would also be good to do some research if there is some existing convention from other actions that we could align ourselves to.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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?

Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, base it on docs seems like the best option imo

Copy link
Owner

Choose a reason for hiding this comment

The 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:

  1. Upper case snake case:
  2. Lower case snake case:
  3. Kebab case:

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.

};