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 typo in dictionary #387

Merged
merged 2 commits into from
Dec 14, 2021
Merged

Fix typo in dictionary #387

merged 2 commits into from
Dec 14, 2021

Conversation

steffahn
Copy link
Contributor

No description provided.

@epage epage merged commit 2748d6a into crate-ci:master Dec 14, 2021
@epage
Copy link
Collaborator

epage commented Dec 14, 2021

Thanks!

@steffahn
Copy link
Contributor Author

This commits lint CI failure seems highly annoying. Results in an additional email about something that isn’t even a problem.

@epage
Copy link
Collaborator

epage commented Dec 14, 2021

New version is released.

@epage
Copy link
Collaborator

epage commented Dec 14, 2021

This commits lint CI failure seems highly annoying. Results in an additional email about something that isn’t even a problem.

It helped me know to squash the commit so it would follow our pattern for making it easier to browse history and generate changelogs.

Unsure of a better way at this time.

@steffahn
Copy link
Contributor Author

At the very least I’d expect a clear heads-up about this in the CONTRIBUTING.md so I’m less surprised by the CI failure notification.

@steffahn
Copy link
Contributor Author

In other words: I didn’t like the experience of getting an unexpected email claiming “PR run failed”, then having to click through it, interpret a hard to understand error message

/usr/bin/docker run --name a68254be57f7673e64c7e943a02042cbf123f_3b0740 --label 6a6825 --workdir /github/workspace --rm -e HOME -e GITHUB_JOB -e GITHUB_REF -e GITHUB_SHA -e GITHUB_REPOSITORY -e GITHUB_REPOSITORY_OWNER -e GITHUB_RUN_ID -e GITHUB_RUN_NUMBER -e GITHUB_RETENTION_DAYS -e GITHUB_RUN_ATTEMPT -e GITHUB_ACTOR -e GITHUB_WORKFLOW -e GITHUB_HEAD_REF -e GITHUB_BASE_REF -e GITHUB_EVENT_NAME -e GITHUB_SERVER_URL -e GITHUB_API_URL -e GITHUB_GRAPHQL_URL -e GITHUB_REF_NAME -e GITHUB_REF_PROTECTED -e GITHUB_REF_TYPE -e GITHUB_WORKSPACE -e GITHUB_ACTION -e GITHUB_EVENT_PATH -e GITHUB_ACTION_REPOSITORY -e GITHUB_ACTION_REF -e GITHUB_PATH -e GITHUB_ENV -e RUNNER_OS -e RUNNER_ARCH -e RUNNER_NAME -e RUNNER_TOOL_CACHE -e RUNNER_TEMP -e RUNNER_WORKSPACE -e ACTIONS_RUNTIME_URL -e ACTIONS_RUNTIME_TOKEN -e ACTIONS_CACHE_URL -e GITHUB_ACTIONS=true -e CI=true -v "/var/run/docker.sock":"/var/run/docker.sock" -v "/home/runner/work/_temp/_github_home":"/github/home" -v "/home/runner/work/_temp/_github_workflow":"/github/workflow" -v "/home/runner/work/_temp/_runner_file_commands":"/github/file_commands" -v "/home/runner/work/typos/typos":"/github/workspace" 6a6825:4be57f7673e64c7e943a02042cbf123f
Linting commits:
* bf5ef31 Code-gen the dictionary
* f6897b0 Fix typo in dictionary
Against 'committed.toml':
ignore_author_re = 'dependabot'
subject_length = 50
subject_capitalized = true
subject_not_punctuated = true
imperative_subject = true
no_fixup = true
no_wip = true
hard_line_length = 0
line_length = 72
style = 'conventional'
allowed_types = [
    'fix',
    'feat',
    'chore',
    'docs',
    'style',
    'refactor',
    'perf',
    'test',
]
merge_commit = false

If this fails, don't sweat it. We're trying to encourage clear communication and not hinder contributions.
If it is a reasonable issue and you lack time or feel uncomfortable fixing it yourself,
let us know and we can mentor or fix it.
bf5ef31d6ee5b94543ecf4f2f5d42aacfb4f19ca: error Invalid commit format invalid commit format
f6897b095c6854e91cf7bced1121fe951dc9a49e: error Invalid commit format invalid commit format

which takes a while to read and make sense of (especially if you don’t start reading at the bottom) only to figure out that this is about the first word of the commit message AND something that’s probably even expected to fail in a PR during review.

@steffahn
Copy link
Contributor Author

steffahn commented Dec 14, 2021

Actually.. maybe – since CI needs to be approved for first-time contributors anyways (right?) – all that’s necessary is that you write an informative comment that there’s expected CI failure (and that it isn’t necessary for the contributor to act upon this failure) due to commit names before you actually start/activate the CI? So post something like

Our CI checks for a specific format of commit messages. Expect a failure of “Lint Commits workflow run” shortly. The exact format of commit messages necessary is unfortunately not yet fully documented; you do not need to fix this yourself if you don’t want to.

before you activate the workflow ;-) [A bot writing such a message would be okay, too.]

(And feel free to adapt/change this if there actually is some documentation somewhere of the expected format; I couldn’t find anything.)

@epage
Copy link
Collaborator

epage commented Dec 14, 2021

Check out this failure

  • I've enabled colored output in CI
  • I've cleaned up the error message from the oh so helpful "error Invalid commit format invalid commit format" to "error Commit is not in Conventional format: Missing type in the commit summary, expected type: description"

I am looking into a welcome bot for a hybrid message between yours and the message I have in the failure message

The exact format of commit messages necessary is unfortunately not yet fully documented

What do you mean? Just that we didn't specify it in the contributing? I'm updating that. The format itself is documented and the output message shows all of the other expectations beyond that we expect.

@steffahn
Copy link
Contributor Author

steffahn commented Dec 14, 2021

Thanks a lot for taking this feedback into account ❤️


  • I've cleaned up the error message from the oh so helpful "error Invalid commit format invalid commit format" to "error Commit is not in Conventional format: Missing type in the commit summary, expected type: description"

Perhaps the error message should also point out which of the commits is the one triggering the error? 😉

Oh, it actually does that... that took me a while connecting the short form 9ec1587 in the list at the top to the full hash at the beginning of the line (my brain initially just completely ignored that initial part of the line). Could the error message be reformatted so that it repeats the whole "9ec1587 chore Update dependencies" inside of the error message, too? Or maybe something like expected `<type>[optional scope]: <description>`, found `chore Update dependencies` could make sense? (I don't know how customizable these messages are.) It should also probably start with Commit message is not in Conventional format, instead of Commit is not in Conventional format

What do you mean? Just that we didn't specify it in the contributing? I'm updating that. The format itself is documented and the output message shows all of the other expectations beyond that we expect.

Ah, I wasn't aware that this was some kind of "standard" format. As mentioned, I didn't actually know if this was documented, hence the note

(And feel free to adapt/change this if there actually is some documentation somewhere of the expected format; I couldn’t find anything.)

I just thought it might not be documented since I assumed it should've been easy to find by looking into CONTRIBUTING if it was documented ^^


I am looking into a welcome bot for a hybrid message between yours and the message I have in the failure message

That would be great!

@epage
Copy link
Collaborator

epage commented Dec 15, 2021

Oh, it actually does that... that took me a while connecting the short form 9ec1587 in the list at the top to the full hash at the beginning of the line (my brain initially just completely ignored that initial part of the line

I've got crate-ci/committed#215 for showing abbreviated IDs.

Or maybe something like expected <type>[optional scope]: <description>, found chore Update dependencies could make sense? (I don't know how customizable these messages are.) It should also probably start with Commit message is not in Conventional format, instead of Commit is not in Conventional format

I'll see what can be done. Part of it is the errors are coming from two parts, the conventional parser (the last part of the message and committed running the parser (the first part of the message). I do know that the parser has some level of context in it, I'll have to see how easy it is to pull that out.

I am looking into a welcome bot for a hybrid message between yours and the message I have in the failure message

That would be great!

I've gone ahead and created crate-ci/committed#214 to track as I'm trying to balance improvements here and on other projects.

@epage
Copy link
Collaborator

epage commented Dec 15, 2021

I also created crate-ci/committed#216 for adding the "actual" part to errors.

@steffahn steffahn deleted the fix-dictionary branch December 15, 2021 02:42
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.

2 participants