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 build.yml to ignore changes that include only .md files #1274

Merged
merged 1 commit into from
Jan 12, 2021

Conversation

gregdek
Copy link
Contributor

@gregdek gregdek commented Jan 11, 2021

That's a lot of CI to run for a PR to a markdown file. This PR should allow build.yml to say "hey, if it's all md files, just, y'know, don't do all that stuff."

I can't think of any cases in which we'd want .md file changes to kick off CI runs, but if you can, feel free to reject this PR.

Also, cheers to the GitHub built-in editor for making sure the yaml is right.

Issue number:

Description of changes:

Testing done:

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

That's a lot of CI to run for a PR to a markdown file. This PR should just say "hey, if it's all md files, just, y'know, don't do that."

I can't think of any cases in which we'd want .md file changes to kick off CI runs, but if you can, feel free to reject this PR.

Also, here's to the GitHub built-in editor for making sure the yaml is right.
Copy link
Contributor

@tjkirch tjkirch left a comment

Choose a reason for hiding this comment

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

Looks good to me; I imagine that if we want CI to run on md files, we'd probably want a rather different Action.

@samuelkarp
Copy link
Contributor

samuelkarp commented Jan 11, 2021

It looks like CI did run for this PR. Is that expected?

I missed the obvious. See @etungsten's response.

@webern
Copy link
Contributor

webern commented Jan 11, 2021

It looks like it should work, based on the documentation. However CI ran for this PR which means it did not work as desired.

@etungsten
Copy link
Contributor

The CI ran for this PR because the change is against a .yml file and the we're ignoring .md files

@webern
Copy link
Contributor

webern commented Jan 11, 2021

The CI ran for this PR because the change is against a .yml file and the we're ignoring .md files

🎓

@tjkirch tjkirch merged commit aa5cf1f into develop Jan 12, 2021
@tjkirch tjkirch deleted the gregdek-patch-2 branch January 12, 2021 21:28
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.

5 participants