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

Add `tidy` to dependencies' CI #10636

Closed
edunham opened this issue Apr 15, 2016 · 8 comments
Closed

Add `tidy` to dependencies' CI #10636

edunham opened this issue Apr 15, 2016 · 8 comments
Assignees

Comments

@edunham
Copy link
Contributor

@edunham edunham commented Apr 15, 2016

Creating a new issue to follow up on #861, since the packaging-tidy discussion took over that thread.

The servo_tidy package is now available on pip at https://pypi.python.org/pypi/servo_tidy/0.0.1 . Currently changes to it need to be deployed manually (secrets are in the "servo build machine secrets" doc), but we can automate the process once it becomes clearer how often we'll need to bump the version.

From examining the cargo.toml, I get this list of external dependencies that we should try to get tidy run on:
Ours (do these first)

Others' (Discuss adding lint to their CI, or alternately run it ourselves)

@jdm
Copy link
Member

@jdm jdm commented Apr 15, 2016

I'm kind of ambivalent about asking authors of crates that are unrelated to Servo (except that we make use of them) to install our Servo-specific linter. For the license checks in particular, I wonder if we could make use of the crates.io API and fetch that data for local evaluation?

@edunham
Copy link
Contributor Author

@edunham edunham commented Apr 15, 2016

@jdm Good point. Do you think it'd be an appropriate compromise to ask the crate authors whether they'd be interested in running our lints, and if they aren't, run our own nightly check on clones of their repos to stay informed of how they compare? I expect that some maintainers will welcome the extra tests while others may disagree with the opinions enforced by the lints.

@jdm
Copy link
Member

@jdm jdm commented Apr 15, 2016

Most of our lints are for enforcing our code style rules (along with some common code nits). I don't think we gain any useful information by doing so, given how little we end up contributing to many of the crates we depend upon.

Another thing to point out is that Cargo.lock has the complete graph of our dependencies, which is a much larger list.

@wafflespeanut
Copy link
Member

@wafflespeanut wafflespeanut commented Apr 16, 2016

This is gonna be fun! 😄

@edunham
Copy link
Contributor Author

@edunham edunham commented Apr 19, 2016

The boilerplate for making tidy run in a non-python and containers+mac test suite is, in .travis.yml,

before_script:
    - export PATH=$HOME/.local/bin:/Users/travis/Library/Python/2.7/bin:$PATH
    - pip install servo_tidy --user `whoami`

script:
    - servo-tidy
bors-servo added a commit to servo/webrender_traits that referenced this issue Apr 19, 2016
Tidy and fixes

for servo/servo#10636

* add tidy stuff to .travis.yml
* transform conditional `use` to one-liners and put them after the other `use` statements
* alphebetize many lists
* add whitespace around some `=`
edunham added a commit to edunham/gaol that referenced this issue Apr 19, 2016
edunham added a commit to edunham/gaol that referenced this issue Apr 19, 2016
edunham added a commit to edunham/ipc-channel that referenced this issue Apr 19, 2016
bors-servo added a commit that referenced this issue Apr 19, 2016
Allow another wording of apache2/MIT (used by gaol)

r? @larsbergstrom

need this for #10636

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10715)
<!-- Reviewable:end -->
@UK992
Copy link
Contributor

@UK992 UK992 commented Aug 26, 2016

@edunham now that tidy's license validation logic and config file landed, it is possible to update servo_tidy on pip?

@edunham
Copy link
Contributor Author

@edunham edunham commented Aug 29, 2016

@UK992 thanks for the reminder! Pushed new version to PyPI and wrote down how to do it this time, https://github.com/servo/servo/wiki/Infrastructure#tidy-on-pypi

@edunham edunham mentioned this issue Aug 29, 2016
1 of 5 tasks complete
bors-servo added a commit that referenced this issue Aug 29, 2016
bump Tidy version

Published a new tidy after config file fixes landed, as requested in #10636 (comment). This commit makes the repo match what I published, is all.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because **I changed one character in a string. If this breaks stuff, we're !#@$#@!$'d.**

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13113)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Aug 30, 2016
bump Tidy version

Published a new tidy after config file fixes landed, as requested in #10636 (comment). This commit makes the repo match what I published, is all.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because **I changed one character in a string. If this breaks stuff, we're !#@$#@!$'d.**

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13113)
<!-- Reviewable:end -->
@edunham edunham mentioned this issue Jul 14, 2017
0 of 3 tasks complete
@edunham edunham self-assigned this Jul 14, 2017
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 1, 2019
…aol) (from edunham:tidy-licenses); r=larsbergstrom

r? larsbergstrom

need this for servo/servo#10636

Source-Repo: https://github.com/servo/servo
Source-Revision: a129ce1bccf313db85547a5c32ecf8cd7aac489b

UltraBlame original commit: 7790d9b954bf74cfa6f43fcbfd7cd371bd5648df
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 1, 2019
…r=emilio

Published a new tidy after config file fixes landed, as requested in servo/servo#10636 (comment). This commit makes the repo match what I published, is all.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because **I changed one character in a string. If this breaks stuff, we're !#$#!$'d.**

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: c4f34e9019e8324440d46a51e3df3bd150507199

UltraBlame original commit: 3fcd631d81f7979e1a5cc6dec75a6609a4d3e2cf
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 1, 2019
…aol) (from edunham:tidy-licenses); r=larsbergstrom

r? larsbergstrom

need this for servo/servo#10636

Source-Repo: https://github.com/servo/servo
Source-Revision: a129ce1bccf313db85547a5c32ecf8cd7aac489b

UltraBlame original commit: 7790d9b954bf74cfa6f43fcbfd7cd371bd5648df
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 1, 2019
…aol) (from edunham:tidy-licenses); r=larsbergstrom

r? larsbergstrom

need this for servo/servo#10636

Source-Repo: https://github.com/servo/servo
Source-Revision: a129ce1bccf313db85547a5c32ecf8cd7aac489b

UltraBlame original commit: 7790d9b954bf74cfa6f43fcbfd7cd371bd5648df
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 1, 2019
…r=emilio

Published a new tidy after config file fixes landed, as requested in servo/servo#10636 (comment). This commit makes the repo match what I published, is all.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because **I changed one character in a string. If this breaks stuff, we're !#$#!$'d.**

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: c4f34e9019e8324440d46a51e3df3bd150507199

UltraBlame original commit: 3fcd631d81f7979e1a5cc6dec75a6609a4d3e2cf
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 1, 2019
…r=emilio

Published a new tidy after config file fixes landed, as requested in servo/servo#10636 (comment). This commit makes the repo match what I published, is all.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because **I changed one character in a string. If this breaks stuff, we're !#$#!$'d.**

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: c4f34e9019e8324440d46a51e3df3bd150507199

UltraBlame original commit: 3fcd631d81f7979e1a5cc6dec75a6609a4d3e2cf
@jdm
Copy link
Member

@jdm jdm commented Jul 29, 2020

This work isn't going to happen.

@jdm jdm closed this Jul 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.