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

[ci] Move check-docs and lint jobs off Travis #3726

Merged
merged 18 commits into from Jan 7, 2021
Merged

Conversation

jameslamb
Copy link
Collaborator

@jameslamb jameslamb commented Jan 4, 2021

Because of #3519 , we're in the process of removing all CI jobs that run on Travis and moving them to GitHub Actions.

This PR moves check-docs and lint to GitHub Actions. I wanted to do this one by itself to make #3672 smaller. These two jobs are also the easiest to move.

This PR proposes putting check-docs and lint into a separate GitHub Actions workflow called "Static Analysis". Today, GitHub Actions does not allow you to re-run individual failed jobs. So if one job fails for random or temporary reasons, you have to re-run all jobs in the workflow. Since the check-docs tasks fails somewhat regularly due to temporary network problems, I think it is a good idea to keep it + lint separate, so that the cost of re-running it is low.

Thanks for reviewing all of these @StrikerRUS . I know it's a lot of PRs but hopefully the end result will be that our CI is more stable.

Copy link
Collaborator

@StrikerRUS StrikerRUS left a comment

Choose a reason for hiding this comment

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

Thanks for doing this! Please check some my initial comments below:

.github/workflows/static_analysis.yml Outdated Show resolved Hide resolved
.github/workflows/static_analysis.yml Outdated Show resolved Hide resolved
.github/workflows/static_analysis.yml Outdated Show resolved Hide resolved
.github/workflows/static_analysis.yml Show resolved Hide resolved
.github/workflows/static_analysis.yml Show resolved Hide resolved
@jameslamb
Copy link
Collaborator Author

ok I think this is ready for another review

Copy link
Collaborator

@StrikerRUS StrikerRUS left a comment

Choose a reason for hiding this comment

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

Generally this R looks OK to me. Thanks! Please check one more round of review below:

.github/workflows/static_analysis.yml Outdated Show resolved Hide resolved
.github/workflows/static_analysis.yml Show resolved Hide resolved
.github/workflows/static_analysis.yml Outdated Show resolved Hide resolved
.github/workflows/static_analysis.yml Show resolved Hide resolved
.github/workflows/static_analysis.yml Outdated Show resolved Hide resolved
.github/workflows/static_analysis.yml Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
.github/workflows/static_analysis.yml Outdated Show resolved Hide resolved
.github/workflows/static_analysis.yml Outdated Show resolved Hide resolved
Copy link
Collaborator

@StrikerRUS StrikerRUS left a comment

Choose a reason for hiding this comment

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

Great! Thank you for reorganizing CI!
I even had a chance to test rerun-all button for new workflow - Wikipedia was not answering. Everything was OK.

Just two nits.

README.md Outdated Show resolved Hide resolved
Comment on lines 39 to 40
export OS_NAME="linux"
export COMPILER="gcc"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can move these variables to env section of this config file closer to PYTHON_VERSION and other constants.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed in 1af56f4

@StrikerRUS StrikerRUS merged commit eb9bbfb into master Jan 7, 2021
@StrikerRUS StrikerRUS deleted the ci/lint-and-docs branch January 7, 2021 17:53
jameslamb added a commit that referenced this pull request Jan 13, 2021
* [ci] move CI jobs from Travis to Azure DevOps (fixes #3519)

* comment out other CIs to avoid wasting cycles

* try without docker

* add container back

* stop using --user in pip install

* run check-docs and lint without container

* job names

* move more jobs to Azure-hosted Linux pool

* fix PATH for check-docs

* uncomment other CI

* uncomment windows

* remove uses of maxParallel

* try moving macos-latest jobs to GitHub Actions

* fix config

* fix missing conda env

* set Python version

* remove commented-out code

* add more to GitHub Actions

* try to fix GPU

* remove static_analysis to prevent conflicts with #3726

* change workflow name

* try using ubuntu:latest docker

* fix conda

* trying to find where permissions first break

* add workaround for sudo

* please azure please

* image syntax

* more sudo

* noninteractive

* LC_ALL

* more  sudo

* more stuff

* CONDA dir

* paths

* get path

* missing CONDA

* fix path stuff

* more tests

* fix graphviz

* stuff

* more graphviz

* install xorg-libxau

* graphviz works, run more jobs

* stuff

* enable more tests

* uncomment GitHub Actions

* uncomment all other CIs

* Apply suggestions from code review

Co-authored-by: Nikita Titov <nekit94-08@mail.ru>

* add travis.yml to Rbuildignore

* add Rbuildignore rule for fmt

* add libomp for clang builds

* changes from code review

Co-authored-by: Nikita Titov <nekit94-08@mail.ru>
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants