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

Transferring eslint-canary to the eslint GitHub organization #8032

Closed
not-an-aardvark opened this issue Feb 6, 2017 · 13 comments
Closed

Transferring eslint-canary to the eslint GitHub organization #8032

not-an-aardvark opened this issue Feb 6, 2017 · 13 comments
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion infrastructure Relates to the tools used in the ESLint development process

Comments

@not-an-aardvark
Copy link
Member

To address #7874, I started the eslint-canary repo, which automatically runs ESLint on a set of existing projects to spot potential regressions. I would be fine with maintaining it myself, but if we want to use it as as part of the project (e.g. as a CI build), it might be useful for other team members to be able to maintain it. So I think it would be a good idea to transfer the repo to the eslint organization.

Does anyone else have thoughts on this? Potential downsides include imposing a maintenance burden on the team, but personally I think it would be much better for everyone to have access to it if we end up running it in CI.

cc @eslint/eslint-team

@not-an-aardvark not-an-aardvark added evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion infrastructure Relates to the tools used in the ESLint development process labels Feb 6, 2017
@mikesherov
Copy link
Contributor

Huge 👍 for me. Esprima does this as well, and it's proven invaluable in catching regressions in downstream projects.

@vitorbal
Copy link
Member

vitorbal commented Feb 6, 2017 via email

@markelog
Copy link
Member

markelog commented Feb 7, 2017

Sounds good to me, it's interesting that there is

There are several camelcase errors in the vue codebase for some reaso

or

There are several semi errors in the webpack codebase for some reason

in project configs :)

@platinumazure
Copy link
Member

We have 5 TSC (if we include @not-an-aardvark) proposing or endorsing the original comment. Assuming no one else in TSC objects, I think that might be sufficient to move forward on this.

@nzakas @ilyavolodin @alberto Do you have any objections to this?

@nzakas
Copy link
Member

nzakas commented Feb 7, 2017

Just my usual concerns about maintenance. We need someone to be the primary maintainer if that repo even if it does move to the ESLint org, as we can't expect everyone who participates in this repo to automatically help maintain that one.

I'd also like to see a proposal for we decide what projects to run there. We obviously don't want to allow an infinite number, so what are the trade offs? Should we require a TSC vote for any additions?

Also, how does this relate to the existing regression builds we have? And what does it mean if the canary build breaks? Part of the problem we've had with our regression builds is that once it breaks, it tends to stay broken for a while, and that makes it less useful.

This should be discussed at a TSC meeting to finalize (we should do that whenever we propose adding a new code repo to the org).

@nzakas
Copy link
Member

nzakas commented Feb 7, 2017

Oh, just a logistical thing: if we adopt this repo, there's a form to fill out to transfer IP ownership to the JS Foundation.

@not-an-aardvark
Copy link
Member Author

not-an-aardvark commented Feb 7, 2017

We need someone to be the primary maintainer if that repo even if it does move to the ESLint org, as we can't expect everyone who participates in this repo to automatically help maintain that one.

I would be willing to be the primary maintainer.

I'd also like to see a proposal for we decide what projects to run there. We obviously don't want to allow an infinite number, so what are the trade offs? Should we require a TSC vote for any additions?

I think we could be very liberal about adding projects. The advantage of adding a project is that we have more code to test against, and the only disadvantage I can think of is that the build takes slightly longer (as well as the small chance of causing a build to fail if a repo disappears from GitHub). I think we definitely should not require a TSC vote for the addition of a project -- my thinking was that if someone makes a PR to add their project and the build still passes, we would generally accept the PR.

Also, how does this relate to the existing regression builds we have? And what does it mean if the canary build breaks? Part of the problem we've had with our regression builds is that once it breaks, it tends to stay broken for a while, and that makes it less useful.

This is independent from the existing regression build, although it has similar functionality. In theory, if we decide to use this, we could eventually stop running the existing regression build, but there's no reason they can't exist simultaneously.

Since cloned projects are pinned to a particular commit, I think the build would generally only fail as a result of changes in ESLint (e.g. a bugfix that causes more errors to be reported), unless the project in question is deleted from GitHub. If errors start getting reported for a project, we can fix it by disabling the given rule for that project. (If a change causes errors to be reported for a very large number of projects, this would also give us more information about the degree of ecosystem breakage caused by the change.)

@mysticatea
Copy link
Member

Also, how does this relate to the existing regression builds we have? And what does it mean if the canary build breaks? Part of the problem we've had with our regression builds is that once it breaks, it tends to stay broken for a while, and that makes it less useful.

I think differences are:

  • I can use eslint-canary on my local PC. This is useful when I made a large change like Update: rewrite TokenStore (fixes #7810) #7936.
  • I can send PR when eslint-canary is broken without regression in a rare case. The recovery way of existing regression build is unclear.

@nzakas
Copy link
Member

nzakas commented Feb 9, 2017

I think we could be very liberal about adding projects.

I'm concerned about resources required to run the project. For instance, if we run this on our Jenkins server, and only one job can be run at a time, that blocks the server for other uses. Or are you planning on using some other service to run this?

In still if the mindset that we need to keep the project list relatively small. I don't think a liberal contribution policy here is a good idea. I'd much rather see high-profile projects like React, Ember, and Node.js than let anyone add their project just because they want to be included. Time is a valuable resource, and I'd prefer every second this runs to be giving us valuable data rather than redundant data.

Also, just to be clear, the intent of this project is to verify latest ESLint changes against projects that use ESLint, but not against integrations like babel-eslint?

@not-an-aardvark
Copy link
Member Author

Another option would be to run the build on Travis. We could have it run at the same time as the CI suite, or alternatively we could try one of the new Travis cron jobs.

Also, just to be clear, the intent of this project is to verify latest ESLint changes against projects that use ESLint, but not against integrations like babel-eslint?

It can also run against projects that use babel-eslint, because it will automatically download dependencies. However, it won't directly verify integrations like babel-eslint, say, by running the babel-eslint test suite.

@not-an-aardvark not-an-aardvark added the tsc agenda This issue will be discussed by ESLint's TSC at the next meeting label Feb 14, 2017
@not-an-aardvark
Copy link
Member Author

not-an-aardvark commented Feb 14, 2017

TSC Summary: eslint-canary is a regression build which clones a list of projects that use ESLint, and lints them to make sure no unexpected errors are reported. In order to maintain the build and run it in CI, it might be useful to move the eslint-canary repo to the eslint GitHub organization.

If we do adopt the repo, we should also decide on criteria for when to add projects to the regression build. Adding more projects will generally increase the chance of finding a regression, but it will also make the build take longer, and block the Jenkins server for longer if we run it on there.

TSC Question: Should we adopt the eslint-canary repo? If so, how should we decide what projects to run in the regression build?

@platinumazure
Copy link
Member

I like the current list of projects in eslint-canary and think it's a great list to start off with.

However, there are three classes of projects missing from eslint-canary right now, and it would be good to include them eventually:

  • Plugins with custom rules
  • Plugins with processors
  • Plugins which export large or complicated configurations

I look forward to seeing how eslint-canary works within the organization. Hopefully we can add a few more projects in the future.

@alberto
Copy link
Member

alberto commented Feb 23, 2017

Resolution:

  • We will move the eslint-canary repository into the ESLint organization.
  • Run the tests nightly to begin with and keep the project list static.
  • Revisit after we've tried it out for a few weeks.

@alberto alberto closed this as completed Feb 23, 2017
@alberto alberto added accepted There is consensus among the team that this change meets the criteria for inclusion and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion tsc agenda This issue will be discussed by ESLint's TSC at the next meeting labels Feb 23, 2017
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 6, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion infrastructure Relates to the tools used in the ESLint development process
Projects
None yet
Development

No branches or pull requests

8 participants