Skip to content

Conversation

@mehtamohit013
Copy link

@mehtamohit013 mehtamohit013 commented Jun 14, 2023

Describe your changes

When the PR is merged into master, CI is not running.
Explanation

Issue number

Closes #X

Checklist before requesting a review


📚 Documentation preview 📚: https://jupysql--617.org.readthedocs.build/en/617/

@mehtamohit013 mehtamohit013 added the allow-workflow-edits Allow edits to .github/workflows label Jun 14, 2023
@mehtamohit013 mehtamohit013 mentioned this pull request Jun 14, 2023
4 tasks
@mehtamohit013
Copy link
Author

mehtamohit013 commented Jun 14, 2023

I tested it out on ploomber dummy repository:

Case-1

  • Created a new branch CI
  • Pushed to that branch
  • Actions

Case-2

  • Create a Fork
  • Commit
  • Make a PR
  • Actions

Working as intended

@mehtamohit013 mehtamohit013 marked this pull request as ready for review June 14, 2023 17:55
else
echo "This is not a pull request event"
echo "Running all tests"
exit 1

Choose a reason for hiding this comment

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

I guess exit 1 will make the action failure?
Should we do exit 0?

Ref

Copy link
Author

Choose a reason for hiding this comment

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

To run all tests, we want check_doc to fail. The test main purpose to determine whether anything other than doc has been modified. The test will fail if anything other than doc was modified and will run all tests.

Choose a reason for hiding this comment

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

OK just saw continue-on-error: true
So I guess that still marks the failing step as "succeeded"?

Copy link
Author

Choose a reason for hiding this comment

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

continue-on-error: true make sure that the jobs below will run irrespective of the failure or success of a step.

I am using the statement the outcome of the step to determine whether the next test will run or not.
if: needs.check_doc.outputs.check_doc_modified == 'failure'

Choose a reason for hiding this comment

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

I see, logically it looks good to me.
Need to merge to master to verify

@tonykploomber tonykploomber self-requested a review June 14, 2023 21:21
@mehtamohit013 mehtamohit013 merged commit a9c5683 into ploomber:master Jun 14, 2023
@mehtamohit013 mehtamohit013 deleted the fix_CI branch June 19, 2023 19:17
@mehtamohit013 mehtamohit013 mentioned this pull request Jun 19, 2023
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

allow-workflow-edits Allow edits to .github/workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants