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

feat: improve anotation message #97

Merged
merged 1 commit into from Aug 23, 2021
Merged

feat: improve anotation message #97

merged 1 commit into from Aug 23, 2021

Conversation

satoren
Copy link
Contributor

@satoren satoren commented Aug 20, 2021

recreate #96. I couldn't reopen it because I rebased it.

annotation example

I need help with the two comment places.

dist/index.js Outdated
@@ -10536,7 +10534,7 @@ const createGithubAccessToken = async (githubAppToken) => {

const response = await makeRequest(query, variables);

if (!response.data && !response.data.createGithubAccessToken.success) {
if (!response.data || !response.data.createGithubAccessToken.success) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I execute yarn build, it will contain things that are not my changes.

Copy link
Member

Choose a reason for hiding this comment

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

let me try to rebuild the master, maybe it was built not with newest version

Copy link
Member

Choose a reason for hiding this comment

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

everything should be fine now, you should merge with newest master


const message = `file=${file}::${formattedLines} ${linesWord} not covered with tests`;

// NOTE: consider an option to show lines directly by attaching 'line' param
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 don't understand the case of this comment. Under what conditions does it occur?

Copy link
Member

Choose a reason for hiding this comment

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

I think this is more an issue for Node.js applications coverage reports, mentioned here #40

I think it's great to remove this comment, I will check one more if we can get it fixed before running this action.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. I'll revert back to comment.

@satoren satoren marked this pull request as ready for review August 23, 2021 13:32
@vitaliimelnychuk
Copy link
Member

Looks good, thanks for your contribution!

@vitaliimelnychuk vitaliimelnychuk merged commit 12ba55c into barecheck:master Aug 23, 2021
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.

None yet

2 participants