Skip to content

Conversation

dhruv
Copy link
Contributor

@dhruv dhruv commented Dec 15, 2020

Solves #20467: Move linter to Cirrus-CI as Travis-CI.org is shutting down

Copy link
Member

@maflcko maflcko left a comment

Choose a reason for hiding this comment

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

ACK, just some nits

@DrahtBot
Copy link
Contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

Copy link
Contributor Author

@dhruv dhruv left a comment

Choose a reason for hiding this comment

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

Comments addressed. Ready for re-review.

Copy link
Member

@maflcko maflcko left a comment

Choose a reason for hiding this comment

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

re-ACK

@hebasto
Copy link
Member

hebasto commented Dec 15, 2020

Concept ACK.

Copy link
Contributor Author

@dhruv dhruv left a comment

Choose a reason for hiding this comment

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

Comments addressed. Ready for review.

@maflcko
Copy link
Member

maflcko commented Dec 16, 2020

ACK 739d390

@dhruv
Copy link
Contributor Author

dhruv commented Dec 17, 2020

Ready for re-review.

@@ -62,6 +62,7 @@ task:
<< : *BASE_TEMPLATE
container:
image: ubuntu:bionic # For python 3.6, oldest supported version according to doc/dependencies.md
cpu: 1 # Cut bill in half for linting
Copy link
Member

Choose a reason for hiding this comment

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

No need for that comment. Can remove or just say 1 is sufficient

Suggested change
cpu: 1 # Cut bill in half for linting
cpu: 1
Suggested change
cpu: 1 # Cut bill in half for linting
cpu: 1 # No need for more CPUs

Copy link
Member

Choose a reason for hiding this comment

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

btw, based on this plot https://cirrus-ci.com/task/4624095184683008

memory can also be set to 1

@maflcko
Copy link
Member

maflcko commented Dec 17, 2020

ACK 4045a67

@maflcko maflcko merged commit 6f2ca72 into bitcoin:master Dec 17, 2020
@hebasto
Copy link
Member

hebasto commented Dec 17, 2020

On my personal account having some "fatal: Not a valid object name master" warnings:

@hebasto
Copy link
Member

hebasto commented Dec 17, 2020

Also in the merge commit, https://cirrus-ci.com/task/6086189209878528?command=lint#L50:

Skipping shell linting since shellcheck is not installed.

@bitcoin bitcoin deleted a comment from curtcurt380 Dec 17, 2020
@maflcko
Copy link
Member

maflcko commented Dec 17, 2020

Missing packages are installed in #20682

@dhruv
Copy link
Contributor Author

dhruv commented Dec 17, 2020

Thanks for catching those, @hebasto. I should've looked closer.

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Dec 17, 2020
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Dec 17, 2020
maflcko pushed a commit that referenced this pull request Dec 21, 2020
3c2478c ci: Print COMMIT_RANGE to the log as it was in Travis CI (Hennadii Stepanov)
c123892 ci: Drop Travis-specific workaround for shellcheck (Hennadii Stepanov)
10af252 ci: Drop Travis-specific way to set COMMIT_RANGE variable (Hennadii Stepanov)
93504da ci: Fix COMMIT_RANGE variable value for PRs (Hennadii Stepanov)

Pull request description:

  This PR:
  - is a #20658 and #20682  followup
  - set the `COMMIT_RANGE` variable correctly for PRs
  - cleans up Travis-specific code
  - prints COMMIT_RANGE value to the log for convenience as it was in Travis CI

ACKs for top commit:
  MarcoFalke:
    ACK 3c2478c

Tree-SHA512: beb933352b10fd5eb3e66373ddb62439e4f3a03b50fb037ee89fa92c0706cec41d05f2d307f15bb18d1e634e6464f4e123b7e2f88703c8edfd145d8d6eff0b1a
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Dec 21, 2020
3c2478c ci: Print COMMIT_RANGE to the log as it was in Travis CI (Hennadii Stepanov)
c123892 ci: Drop Travis-specific workaround for shellcheck (Hennadii Stepanov)
10af252 ci: Drop Travis-specific way to set COMMIT_RANGE variable (Hennadii Stepanov)
93504da ci: Fix COMMIT_RANGE variable value for PRs (Hennadii Stepanov)

Pull request description:

  This PR:
  - is a bitcoin#20658 and bitcoin#20682  followup
  - set the `COMMIT_RANGE` variable correctly for PRs
  - cleans up Travis-specific code
  - prints COMMIT_RANGE value to the log for convenience as it was in Travis CI

ACKs for top commit:
  MarcoFalke:
    ACK 3c2478c

Tree-SHA512: beb933352b10fd5eb3e66373ddb62439e4f3a03b50fb037ee89fa92c0706cec41d05f2d307f15bb18d1e634e6464f4e123b7e2f88703c8edfd145d8d6eff0b1a
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants