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

Doc: Continuous integration information #13995

Merged
merged 7 commits into from
May 14, 2021
Merged

Doc: Continuous integration information #13995

merged 7 commits into from
May 14, 2021

Conversation

tupui
Copy link
Member

@tupui tupui commented May 6, 2021

In the effort to improve contributing documentation (#12633), this adds information about CI.

Also fix a missing command in a quickstart guide.

Feel free to add more information and complete this 😃

@tupui tupui added Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org CI Items related to the CI tools such as CircleCI, GitHub Actions or Azure labels May 6, 2021
@tupui
Copy link
Member Author

tupui commented May 6, 2021

I can also see that skipping multiple CI works properly 😅 I will just add to put this on a new line to make it clean (as this last commit).

@andyfaff
Copy link
Contributor

andyfaff commented May 6, 2021

I'm not sure that we want to write a table of what we check in CI, this changes regularly.
Perhaps the biggest aspect that might help is to ask users to run the relevant test suite locally before making a commit on a PR.

@tupui
Copy link
Member Author

tupui commented May 6, 2021

I'm not sure that we want to write a table of what we check in CI, this changes regularly.
Perhaps the biggest aspect that might help is to ask users to run the relevant test suite locally before making a commit on a PR.

That's why I tried to not list all of them and wrote matrix for some.

For local testing, I can put a line and refer to the existing doc and the checklist (EDIT: done)

Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Thanks @tupui. This is a good start. Some comments to keep it more maintainable.

You are still missing some of the key we want for this section I think: how to run things locally if CI fails. In particular for the custom steps, like refguide check and linting. Can you add the exact invocations to run those, or better - add them in http://scipy.github.io/devdocs/dev/contributor/runtests.html#runtests and link to those?

doc/source/dev/contributor/continuous_integration.rst Outdated Show resolved Hide resolved
doc/source/dev/contributor/continuous_integration.rst Outdated Show resolved Hide resolved
doc/source/dev/contributor/continuous_integration.rst Outdated Show resolved Hide resolved
doc/source/dev/contributor/continuous_integration.rst Outdated Show resolved Hide resolved
doc/source/dev/contributor/continuous_integration.rst Outdated Show resolved Hide resolved
doc/source/dev/contributor/continuous_integration.rst Outdated Show resolved Hide resolved
doc/source/dev/contributor/continuous_integration.rst Outdated Show resolved Hide resolved
@tupui
Copy link
Member Author

tupui commented May 7, 2021

Interesting, CI did not skip this time. The only difference is the blank line. I will force push just removing the blank line to check.

@tupui
Copy link
Member Author

tupui commented May 7, 2021

Confirmed. The blank line is incompatible and prevent the check to work. Also [skip github] is not working.

So I propose to remove the blank line (making it work with the blank line might be too involved for this PR) and use again [skip actions].

@rgommers
Copy link
Member

rgommers commented May 7, 2021

Confirmed. The blank line is incompatible and prevent the check to work. Also [skip github] is not working.

@larsoner is this expected?

@larsoner
Copy link
Member

@larsoner is this expected?

At least for [skip azp] the string comes from git log --max-count=1 --skip=1 --pretty=format:"%s", which just gets the commit "title" (first line) and not the "description" (anything that follows). This can be useful at least when doing a squash+merge so that intermediate commits with [skip azp] don't cause the main build to skip building. Personally I also don't mind putting the skips in the title, but I'm okay with expanding it to look in the description, too.

@rgommers
Copy link
Member

Thanks for explaining @larsoner!

Personally I also don't mind putting the skips in the title, but I'm okay with expanding it to look in the description, too.

That would be great. I think skips naturally go at the end of the description.

@tupui
Copy link
Member Author

tupui commented May 10, 2021

FYI, I just used [skip ci] in gh-14022 and it skipped everything BUT azure.

@rgommers
Copy link
Member

FYI, I just used [skip ci] in gh-14022 and it skipped everything BUT azure.

Yes, that has always been the case. [skip ci] is kind of the universal command (except Azure devs are really dense and refuse to honor it for PRs from forks). The per-provider ones are the pain point.

@tupui
Copy link
Member Author

tupui commented May 13, 2021

Shall I add anything else? Shall I write that the global skip is not working with azure so if you want to skip all you need to also add the instruction for azure?

@rgommers
Copy link
Member

Shall I write that the global skip is not working with azure so if you want to skip all you need to also add the instruction for azure?

That doesn't work either, Azure just refuses to be skipped.

I think it looks good as is.

@rgommers rgommers merged commit 5089970 into scipy:master May 14, 2021
@rgommers
Copy link
Member

Merged, thanks @tupui, all.

@rgommers rgommers added this to the 1.7.0 milestone May 14, 2021
@tupui tupui deleted the doc_ci branch May 26, 2021 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Items related to the CI tools such as CircleCI, GitHub Actions or Azure Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants