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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Azure Pipelines for CI using awesome-lint #1505

Closed
wants to merge 6 commits into from
Closed

Add Azure Pipelines for CI using awesome-lint #1505

wants to merge 6 commits into from

Conversation

damccorm
Copy link

@damccorm damccorm commented Jan 18, 2019

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.

image

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.

image

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.

image

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

  • You have to review at least 2 other open pull requests. Try to prioritize unreviewed PRs, but you can also add more comments to reviewed PRs. Go through the below list when reviewing. This requirement is meant to help make the Awesome project self-sustaining. Comment here which PRs you reviewed. You're expected to put a good effort into this and to be thorough. Look at previous PR reviews for inspiration.

Reviewed #1508, #1499, #1486, and #1452

  • Rest of the requirements don't apply since this isn't an awesome list

@damccorm
Copy link
Author

@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.

@damccorm
Copy link
Author

@sindresorhus now that sindresorhus/awesome-lint#55 is in, would you be open to this?

@transitive-bullshit
Copy link
Contributor

Hey @damccorm, just throwing in my 2 cents as a collaborator on awesome-lint.

  1. The currently most popular free CI for open source projects on GitHub is travis-ci. I understand you're trying to promote your team's product, but I'm not sure I see any advantages of using Azure pipelines over using travis-ci. Can you expand on why an OSS project that's not already fully invested in a Microsoft / Azure stack would consider using Azure? Even though I used to work for Microsoft myself, I'm sure I'm not alone in saying that many in the OSS community tend to shy away from using Azure because it's traditionally more enterprise focused.

  2. travis-ci is pretty simple to setup in terms of just adding a .travis.yml file to repo and logging into travis with your GitHub profile. How would setup and authorization work equivalently for Azure pipelines?

  3. I'm not sure if disabling the blocked linting rule is a good idea, and if it is decided to proceed with a partial awesome-lint CI integration, that decision seems orthogonal to which CI service to use. I just want to make this clear so we discuss the travis-ci versus Azure pipelines separately from this part of the decision.

  4. If you or someone from your team were to use Microsoft resources to help resolve the core underlying issue, that would go a long way in my mind towards a show of good faith and I would be much more open to using Azure pipelines as this PR proposes. This would also just be a huge win for the community in general.

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!! 馃槃

@damccorm
Copy link
Author

damccorm commented Feb 14, 2019

Hey @transitive-bullshit thanks for weighing in! Apologies in advance for the long answer:

  1. I get this hesitation, Travis has been around forever and is probably the only reason CI is at the point its at today. With that said, I (as an admittedly biased source) think that Pipelines offers better value to most of the OSS community at this point in time.

Advantages for this project right now

  • Test reporting UI - This might be my favorite part of Pipelines and I think sets it apart from other offerings. It provides several advantages, but I think the biggest one for this project is that you can link to the results and have a really easy reference for what linting rules need to be fixed (rather than being forced to search through logs). Here's an example of the test logs for Prettier (who interestingly seem to be blocked by the same remark issue)
  • Perf - I don't have the data for this, but I've noticed a lot of instances of Travis failing to queue builds or doing it after hours of waiting. I haven't noticed the same for Pipelines, in large part because we just have a lot of resources (being part of Azure has its benefits 馃槂).

Potential advantages for this project in the future

  • 10 free parallel builds for OSS - this doesn't matter much right now, but down the line if we start running this on all items in the awesome list it would be really good to be able to leverage that parallelism. Each linting job takes ~30 seconds and there are hundreds of lists, so we're talking the difference between 1-2 hour build times and 10 minute build times.

Advantages for OSS in general
These don't necessarily apply to this project but I wanted to include them for completeness

  • Cross platform builds - Windows/Linux/Mac from a single definition
  • Unlimited free build minutes for OSS

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).

  1. Azure Pipelines is integrated into GitHub more or less the same way, so setup is pretty straightforward. The fastest way is probably to just install the Azure Pipelines app from the GitHub marketplace. This will walk you through setting up an organization/project to host it and creating a pipeline. In the future you can just log in to the organization and point it to the right repository similar to Travis. Should take <5 minutes

  2. Agreed, that's a good distinction to make. I would be interested in hearing your hesitations on this though, it feels like a win to me to get some of these checks going.

  3. I'll try to take a look when I get the bandwidth. I can't guarantee that I'll solve it (given how long its been around it seems like a challenging issue), but I agree it would be really good to address - plus lots of the libraries I love use Remark and I'd just enjoy the chance to understand the code base a little bit better.

@damccorm
Copy link
Author

@sindresorhus would you be open to taking a look at this? As @transitive-bullshit pointed out, there are 2 somewhat orthogonal questions here:

  1. Are you open to trying Pipelines given the discussion above?

  2. Is it worth it to proceed with a partial awesome-lint CI solution with the broken rule disabled?

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.

@damccorm
Copy link
Author

@sindresorhus any updates on this?

@sindresorhus
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants