ci: Cleanup macOS runs #17176

1. 
   MarcoFalke commented at 2:52 pm on October 17, 2019: member

   • Remove a commented out cleanup task in before_cache
   • Remove the linter run on macOS, and document that GNU tools are required to run the linters
2. ci: Cleanup macOS runs 4444704ca9
3. doc: Document that GNU tools are required for linters fadccb263b
4. MarcoFalke added the label Tests on Oct 17, 2019
5. ryanofsky approved
6. 
   ryanofsky commented at 3:25 pm on October 17, 2019: member

   Code review ACK fadccb263baf6b8694f750623add42f966e423a3. But since this change reverts #13728, and I’m not sure what the specific goals of #13728 were, I guess I’d only give an uncertain +0.5 concept ACK.

   For the specific scripted diff problem I ran into #13728 (comment), other solutions may make it possible to restore #13728 in the future like using homebrew to install modern versions of sed and bash, or just skipping the scripted-diff lint check on macos but running all the other lint checks.

7. ci: Remove redundant check for TRAVIS_OS_NAME

   Can be reviewed with
   git diff --ignore-all-space --function-context

   fa677d1801
8. 
   MarcoFalke commented at 3:40 pm on October 17, 2019: member

   I’m not sure what the specific goals of #13728 were

   I assumed it was a low-risk change that would provide us with some experience of macOS on travis (to help go forward with #16597). It also had several ACKs #13728 (comment), #13728#pullrequestreview-274735332

9. 
   MarcoFalke commented at 4:56 pm on October 17, 2019: member

   Also, now that #16597 is merged, we should be more cautious with using too much travis resources. On weekdays a backlog of macOS builds up here: https://www.traviscistatus.com/
10. 
    Empact commented at 5:42 pm on October 17, 2019: member

    Believe you could directly revert this merge commit: https://github.com/bitcoin/bitcoin/commit/42d0eca725a83c999e2b67e33dfc7bcc96288dc3

    And an alternative solution to issue noted in #13726 could be to disable the scripted-diff checks in that environment. It’s still the case that people will run Mac OS environments.

11. 
    MarcoFalke commented at 6:24 pm on October 17, 2019: member

    There is a backlog of about 400 jobs on macOS travis right now (see the link in my previous comment). I don’t think it is worth it to use those resources for linters, when mac users can simply not run the linters (they are run on travis as the fist step, real quick) or install GNU sed via brew or run them in a bionic docker container.
12. 
    DrahtBot commented at 6:49 pm on October 17, 2019: member

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

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #17165 ([WIP] Remove BIP70 support by fanquake)

    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.

13. ryanofsky approved
14. 
    ryanofsky commented at 6:57 pm on October 17, 2019: member

    Code review ACK fa677d1801fb9153a95a1fc9855fd5f21fc440c0 for new third commit replacing TRAVIS_OS_NAME check with NO_DEPENDS setting
15. 
    MarcoFalke commented at 7:03 pm on October 17, 2019: member

    Instead of the linter, I’d rather have a depends built on macOS. I’ll leave that for a follow up pull request.
16. 
    MarcoFalke commented at 12:30 pm on October 19, 2019: member

    The linter isn’t particularly stable, it seems: #17183 (comment)

    Any objections to getting this merged. @Sjors ?

17. 
    Empact commented at 1:09 pm on October 19, 2019: member

    Why not revert the merge commit to make it clearer what is reversion and what is clean-up / clarify authorship?
18. 
    MarcoFalke commented at 1:56 pm on October 19, 2019: member

    It is a revert of the merge commit (with conflicts solved) and the two commented out lines removed
19. 
    Empact commented at 2:46 pm on October 19, 2019: member

    Just suggesting that what you report would be more explicitly and succinctly communicated by the changelog if you separated those two operations. May be obviated by the conflicts though, I’m not sure about that.
20. 
    Sjors commented at 9:30 am on October 21, 2019: member

    Code review ACK fa677d1801fb9153a95a1fc9855fd5f21fc440c0

    Disabling the linter probably deserves its own commit / a more clear commit name.

21. 
    laanwj commented at 10:19 am on October 21, 2019: member

    ACK fa677d1801fb9153a95a1fc9855fd5f21fc440c0
22. laanwj referenced this in commit fc1040acc0 on Oct 21, 2019
23. laanwj merged this on Oct 21, 2019
24. laanwj closed this on Oct 21, 2019

    ---

25. MarcoFalke deleted the branch on Oct 22, 2019
26. DrahtBot locked this on Dec 16, 2021

Contributors
MarcoFalke ryanofsky Empact DrahtBot Sjors laanwj

Labels
Tests