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

Prevent non-docs stages when a PR only modifies docs/ #8992

Closed
wants to merge 12 commits into from

Conversation

mikepapadim
Copy link
Contributor

This is an intermediate fix for #8673.
It modifies Jenkinsfile to prevent non-doc related stages from running when only modifying in docs/.
The idea is to save some CI cycles and speed up the process of augmenting docs.

@areusch @comaniac @tqchen

@Hzfengsy
Copy link
Member

Thanks, @mikepapadim. I agree that it is important to save the CI time.

However, tutorials may be changed even though the codes in docs/ are not changed. For example, when we change the printer(files in src/ and include) may still change the document. These changes are not often happened but they do exist.

@areusch
Copy link
Contributor

areusch commented Sep 13, 2021

@mikepapadim this looks much closer to what we want but I think if I'm reading this right, I see:

  • whenever a change only modifies docs/, we skip all build, unit test, integration test steps

as @Hzfengsy said, we shouldn't ever skip these nodes:

  • Sanity Check (this should already be correct in the patch now)
  • BUILD: GPU
  • docs: GPU

i think we need to find a way to propagate these conditions down to the node level in each stage (e.g. skip each node not listed above when changeset "docs/**" doesn't match). i'm not entirely sure how to do this.

@mikepapadim
Copy link
Contributor Author

@mikepapadim this looks much closer to what we want but I think if I'm reading this right, I see:

  • whenever a change only modifies docs/, we skip all build, unit test, integration test steps

as @Hzfengsy said, we shouldn't ever skip these nodes:

  • Sanity Check (this should already be correct in the patch now)
  • BUILD: GPU
  • docs: GPU

i think we need to find a way to propagate these conditions down to the node level in each stage (e.g. skip each node not listed above when changeset "docs/**" doesn't match). i'm not entirely sure how to do this.

Yes, right now the changeset condition is in the stage scope, I will have another look on how to make use of it in node scope.

Copy link
Contributor

@leandron leandron left a comment

Choose a reason for hiding this comment

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

There are a couple unrelated changes to this patch. Apart from that, I agree in principle with the proposed change.

@@ -6,9 +6,9 @@
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
#
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like an unrelated change.

@@ -19,4 +19,3 @@ Include the title of the document (e.g. "Getting Started with TVM"), and the typ
If an RFC/discuss post exists, link it here.

Otherwise, specify what actions should be taken to provide additional clarity/readability/reproducibility to the document. Include code snippets from the previous documentation if applicable.
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like an unrelated change.

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.

4 participants