-
-
Notifications
You must be signed in to change notification settings - Fork 27.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
Add Azure Pipelines for CI using awesome-lint #1505
Conversation
@sindresorhus would you mind taking a look at this when you get a chance? @transitive-bullshit maybe you have some input as well - I know you were doing some work in this area. |
React to awesome-lint changes
@sindresorhus now that sindresorhus/awesome-lint#55 is in, would you be open to this? |
Hey @damccorm, just throwing in my 2 cents as a collaborator on
To be clear, I don't speak for @sindresorhus by any means -- I just wanted to clarify my thoughts on this topic. Regardless of how we proceed, I definitely want to thank you for your interest and contributions thus far!! 😄 |
Hey @transitive-bullshit thanks for weighing in! Apologies in advance for the long answer:
Advantages for this project right now
Potential advantages for this project in the future
Advantages for OSS in general
As an overall note, I agree that Azure/Microsoft has traditionally been more enterprise focused, but that's changing for what its worth. That's part of what's motivating things like the 10 parallel job offering for OSS in Pipelines and on a larger scale, the GitHub acquisition. Lastly, my recommendation would be to just give it a shot. If you don't like it switching back to Travis should be really easy for this repo (just swap Yaml files).
|
@sindresorhus would you be open to taking a look at this? As @transitive-bullshit pointed out, there are 2 somewhat orthogonal questions here:
I was hoping to resolve (2) by solving the issue, but I've been struggling to get much traction - hopefully I've at least helped push it forward a little bit. I'll come back to it when I get the bandwidth, but in the meantime IMO its probably worth it to get some level of CI going (whether Pipelines or Travis) to lighten the review load. Do you agree? @transitive-bullshit I'd be interested in hearing your hesitations on this issue as well. |
@sindresorhus any updates on this? |
Sorry, I totally missed this. While I appreciate the effort, I'm happy with using Travis. It requires less config and I'm used to its interface. If I were to use something else today, I would probably go with GitHub Actions because of its seamless integration into GitHub. |
Description
This PR adds Azure Pipelines to perform CI builds and PR builds using awesome-lint. I know you've had discussions about CI in this repo before in #1365, and you ultimately ran into this blocker. My solution provides all of the advantages discussed and addresses this problem by using a custom config file that ignores the bad rule that was a blocker. For CI builds, it just validates that this repo conforms to awesome-lint, for PR builds it checks if this repo conforms and additionally checks if all added linked GitHub repos in the diff conform.
Full disclosure: I work at Microsoft as a software engineer on Pipelines, but I also genuinely think this could be a really helpful improvement. A couple of things I'd like to highlight:
My pipeline
You can see that this works for me in my pipeline here.
PR validation
I added a PR that links to a list that should fail the linting here. You an see that it failed the status check and links to the full build.
Test reporting
One of the big advantages of Azure Pipelines is that it provides really nice test reporting. Unfortunately, to do this you need to have a formatted test file to draw from which requires a custom reporter. Awesome-lint doesn't currently support this, but I've added a PR to add that capability. Once that's (hopefully) merged in, we can just consume the newest version of awesome-lint and flip on the
publishTests
flag and we'll get test reporting just like this.EDIT: That PR has been merged so we now have fully functional test reporting.
Config file
Right now I'm using a config file for awesome-lint that is basically an exact replica of the one they have by default. The only difference is I commented out the 1 breaking rule. I'm also happy to add or subtract additional rules as appropriate, or this can be fine tuned over time if a rule proves to be a red herring often. In addition, if awesome-lint ever reaches the point where we just want to use that, we can just delete the config file/reference to it in
validate-requirements.js
and it should just work.By submitting this pull request I confirm I've read and complied with the below requirements 🖖
Requirements for your pull request
Reviewed #1508, #1499, #1486, and #1452