-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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] fix errors about line length in yaml files (Part 4) #6804
Conversation
if: | | ||
github.event.issue.pull_request && | ||
contains('OWNER,MEMBER,COLLABORATOR', github.event.comment.author_association) && | ||
startsWith(github.event.comment.body, '/gha run') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -394,6 +414,7 @@ jobs: | |||
- script: | | |||
git clean -d -f -x | |||
displayName: 'Clean source directory' | |||
# yamllint disable rule:line-length |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refer to #6758 (comment)
@@ -63,10 +63,12 @@ jobs: | |||
submodules: true | |||
- name: Install packages | |||
shell: bash | |||
# yamllint disable rule:line-length |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't able to make disable-line
directive work with multiline string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this all looks good to me! I appreciate the comments you left on the diff explaining some of the workarounds .
R_LIBS="c('R6', 'data.table', 'jsonlite', 'knitr', 'markdown', 'Matrix', 'RhpcBLASctl', 'testthat')" | ||
RDscript${{ matrix.r_customization }} \ | ||
-e "install.packages(${R_LIBS}, repos = 'https://cran.rstudio.com', Ncpus = parallel::detectCores())" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok with this for now now. But I think we should eventually just move all these R package installs into a single CI script like ci/install-r-deps.R
, to reduce duplication. Would you support something like that? I can put up a PR if you agree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely, it would be great!
Last PR in series of PRs for linting yaml files.
This PR fixes long lines error.
Continuation of #6794.
See #6758 for background and future PRs' content required to fix all errors with the proposed config.
Closed #6758.