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

Fixed CI (Doc checking) #668

Merged
merged 2 commits into from
Jun 26, 2023
Merged

Fixed CI (Doc checking) #668

merged 2 commits into from
Jun 26, 2023

Conversation

mehtamohit013
Copy link

@mehtamohit013 mehtamohit013 commented Jun 26, 2023

Describe your changes

Correct Syntax: -e doc CHANGELOG.md

Issue number

Closes #661

Checklist before requesting a review


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

@mehtamohit013 mehtamohit013 added no-changelog This PR doesn't require a changelog entry allow-workflow-edits Allow edits to .github/workflows labels Jun 26, 2023
@mehtamohit013
Copy link
Author

mehtamohit013 commented Jun 26, 2023

@edublancas
I have created a seperate branch CI with the changes and then create a new branch doc_modified with only doc and CHANGELOG.md changes here: #667

Also created a PR in pkgmt with changes: ploomber/pkgmt#61

@yafimvo
Copy link

yafimvo commented Jun 26, 2023

@mehtamohit013 do we need to merge the pkgmt PR in order to test it?

Also, I'm not sure I understand this PR (667) . If we changed only docs and/or changelog - No other test should run, right?

according to this summary, it looks like the CI tests will run even when we changed only docs/changelog.

image

If it's possible, I think we should test it as follows

  1. Create a new commit - change something in the changelog -> CI tests shouldn't run
  2. Create a new commit - change something in one of the docs -> CI tests shouldn't run
  3. Case 1 and 2 -> CI tests shouldn't run
  4. Create a new commit - change something in the code -> CI test should run
  5. empty commit

I think we can test all these cases in this PR by modifying the relevant files each time.

@mehtamohit013
Copy link
Author

Hi @yafimvo,
I believe I have tested the same here, in this PR, and it has been successfully merged.

This PR mainly corrects the syntax, which I added earlier: #626

@mehtamohit013
Copy link
Author

Also created a PR in pkgmt with changes: ploomber/pkgmt#61

Also, I have corrected a docstring here, changed no code or logic

@mehtamohit013
Copy link
Author

@yafimvo

do we need to merge the ploomber/pkgmt#61 in order to test it?

No

Also, I'm not sure I understand this #667. If we changed only docs and/or changelog - No other test should run, right?
according to this summary, it looks like the CI tests will run even when we changed only docs/changelog.

As you see, in the summary, the jobs are skipped. Also, you can see Check_doc_modified has passed( which means the doc has been modified) and as a result all the tests are skipped.

@edublancas edublancas merged commit 452674e into ploomber:master Jun 26, 2023
@mehtamohit013 mehtamohit013 deleted the fix_CI branch August 8, 2023 18:37
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 no-changelog This PR doesn't require a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tasks triggered when only modifying changelog and doc/
3 participants