Skip to content

ci: don't run tests when markdown files are changed #170

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

Merged
merged 1 commit into from
Feb 3, 2021

Conversation

ErikSchierboom
Copy link
Member

@ErikSchierboom ErikSchierboom commented Feb 3, 2021

Copy link
Member

@ee7 ee7 left a comment

Choose a reason for hiding this comment

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

Nice.

LGTM, but I'd suggest we amend the commit message to something like

ci: don't run tests workflow for Markdown-only changes

with the body

To prevent the tests workflow from running unnecessarily we don't run it
when only Markdown files are changed

This adds the word "only" to both - the previous wordings imply that we don't run tests for any commit that includes a change to a .md file.

And nitpick: can we wrap the commit message body at 72 chars?

@ErikSchierboom ErikSchierboom force-pushed the runs-test-on-files-changes branch from 7544fe0 to b9ebadd Compare February 3, 2021 12:00
To prevent the tests workflow from running unnecessarily, we don't run it when only Markdown files have changed
@ErikSchierboom ErikSchierboom force-pushed the runs-test-on-files-changes branch from b9ebadd to 6c26194 Compare February 3, 2021 12:01
@ErikSchierboom
Copy link
Member Author

I've updated the commit message.

And nitpick: can we wrap the commit message body at 72 chars?

With due respect, but I won't be doing this. We don't enforce this anywhere on Exercism and I personally don't feel that it offers much.

@ErikSchierboom ErikSchierboom merged commit 968f038 into main Feb 3, 2021
@ErikSchierboom ErikSchierboom deleted the runs-test-on-files-changes branch February 3, 2021 12:08
@ee7
Copy link
Member

ee7 commented Feb 3, 2021

And nitpick: can we wrap the commit message body at 72 chars?

With due respect, but I won't be doing this. We don't enforce this anywhere on Exercism and I personally don't feel that it offers much.

No problem.

By the way, no need to force-push an amended commit if there's zero code change - I'd be fine if you added the "only" while you clicked "squash + merge".

@ErikSchierboom
Copy link
Member Author

Ah okay.

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