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

Replace Travis CI with GitHub Actions in generated Ember CLI projects #696

Closed

Conversation

kellyselden
Copy link
Member

@kellyselden kellyselden commented Jan 4, 2021


## Alternatives

CircleCI and GitLab are other popular alternatives.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is going to be some pain with switching the platforms, it seems better to unify those systems under one roof. EmberJS (core) git projects are hosted on GitHub, so having CI there seems like a logical step.

Copy link
Member Author

@kellyselden kellyselden Jan 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we don't say you have to put your personal project on GitHub, so I think the fact that we use it internally is irrelevant.

@ijlee2
Copy link
Sponsor Member

ijlee2 commented Jan 4, 2021

A similar proposal (#596) was made several months ago, in the form of an issue. Do we want to combine the two somehow?

@kellyselden
Copy link
Member Author

Oh darn I didn't see that. Yes it seems like the same issue. I'd like to avoid the larger scope of that one. It mentions dependabot, code coverage, and new CLI options. This one can be completed much quicker with its limited scope.

GitHub Actions doesn't support retrying individual jobs like Travis CI.
This could impact some user's monthly budget.

## Alternatives
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It misses the alternative to not include any CI configuration in the default blueprints. Instead some tooling could be provided to generate a CI configuration for the target provider (TravisCI, GitHub Actions, GitLab CI, Circle CI etc.). The script could be smart enough to provide sensitive defaults based on the Ember CLI version used. It could also support ugprades of existing CI configuration to the latest best practices. It could be even included in default blueprints and expose an Ember CLI command like ember ci or a blueprint like ember generate ci.

To be honest that would be my preferred solution. create-github-actions-setup-for-ember-addon could be seen as a very early step in that direction.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I added the alternative of not shipping a CI file. As for adding new functionality, I left that out to keep the scope small for this change, since Travis CI is basically inoperable now and fixing this ASAP would be best.

@sukima
Copy link

sukima commented Jan 4, 2021

Alternative: Move the CI scripts into generators thus allowing the developer of a newly generated app to opt-in to the CI they prefer with commands like ember g github-ci, ember g circle-ci, or ember g gitlab-ci.

CircleCI and GitLab are other popular alternatives.
Perhaps one would be better as a recommendation from us?

We could also refrain from suggesting/including any CI config files.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would mean removing .travis.yml from default blueprints without a replacement, wouldn't it? This would be my preferred short-term solution. Additionally we should experiment with alternative solutions to provide an easy setup of CI pipelines. But no need to do this kind of experiments in Ember CLI itself.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reality is that the majority of Ember addons are living on GitHub, and that they have been using TravisCI by default so far. Not including a config means that new addons don't get CI by default, and existing addons might have their CI config removed. That would not be a good solution IMHO.

I agree with Kelly that the MVP here is replacing the TravisCI config with a default GitHub Action config, and if that isn't used because of using some other hosting provider then people can delete it.

This does not mean we should stop considering other options, like the generator that was mentioned. But we need a short-term solution now, and this RFC has a much smaller scope, which makes it much more likely that we can implement it soonish.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reality is that the majority of Ember addons are living on GitHub, and that they have been using TravisCI by default so far.

I agree that this is true for open source addons. But I'm not sure if it is still true if also considering internal addons. We are making heavy use of private addons in the project I'm currently working on. Not using GitHub or TravisCI for it. Not sure how it is for others.

The argument also doesn't take applications into account. CI configuration is currently not only included in addon but also in application blueprints. We are not using GitHub but GitLab at work. I guess many other teams do as well.

Not including a config means that new addons don't get CI by default

Long time since I created a new addon, which was using TravisCI. But if I recall correctly maintainer needed to enable the TravisCI for the new project explicitly. I don't see a big difference between doing that manual step or running a setup script like yarn create-github-actions-for-ember-addon.

and existing addons might have their CI config removed.

The motivation section of this RFC states that the existing CI configuration is unusable. So deleting the .travis.yml when updating Ember CLI blueprints should not make it worth. Additionally nothing prevents a maintainer from not following that blueprint change.

I agree with Kelly that the MVP here is replacing the TravisCI config with a default GitHub Action config, and if that isn't used because of using some other hosting provider then people can delete it.

The file will be recreated by ember-cli-update everytime it changes in default blueprints. For CI configuration this is atleast once per LTS version. It's easy to miss on upgrades. It quickly gets annoying. We stopped deleting the existing .travis.yml file at work therefore. @mehulkar raised the same concern in #596 (comment).

This does not mean we should stop considering other options, like the generator that was mentioned. But we need a short-term solution now, and this RFC has a much smaller scope, which makes it much more likely that we can implement it soonish.

As far as I understand we agree that .travis.yml should be removed from default blueprints. We seems to disagree if a GitHub Actions workflow should be added as a replacement or not. Taking scope into account not proving a replacement as part of the blueprint is smaller than providing one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taking scope into account not proving a replacement as part of the blueprint is smaller than providing one.

it not smaller at all, because "should we include a CI config at all?" is a much larger discussion than "should we include a CI config for the most used CI service?"

I'm aware that for internal apps and addons different rules apply, but I am primarily worried about public addons not running CI anymore because of the change you propose. If internal apps and addons break because they are not running CI anymore then that's an internal problem, but if public addons stop working then that becomes a problem of everyone.

@kiwiupover
Copy link

@kellyselden thanks heaps for pushing this forward.

@elwayman02
Copy link
Contributor

elwayman02 commented Jan 7, 2021

Great proposal!

One caveat I would like to point out about GitHub Actions, however, is that it does not truly have an "allow-failure" option for testing against beta/canary builds, the way that Travis does. We will have to ensure that our replacement config accounts for this and also do our best as a community to pressure GitHub to support this feature. The best replacement I've seen so far is continue-on-error, which isn't exactly the same but should at least prevent the builds from failing. @kellyselden you will likely want to call this out somewhere in the RFC in terms of drawbacks.

Following up on other conversations above, I strongly prefer that we suggest and provide a solid default option for all projects (as this RFC proposes). There is no benefit to shipping the blueprints without any default CI configuration, because that just creates unnecessary friction. Ember and Ember-CLI as a community is built around shipping solid defaults. We don't refrain from shipping QUnit integrations just because someone might want to use Mocha, nor do we skip out on eslint or prettier because someone might choose another tool. If someone does not want to use the bundled CI config, including that file does not create reasonably more work for them than they were going to have to do anyway. They can simply delete the file and add their own. It does not make sense, however, for us to force ALL developers to have to take the extra step of explicitly creating their CI configs for every single project. It is such an integral part of a project that we would be doing the community a disservice by not including some sane default service configuration.

Given that GitHub's market share is is ~89% compared to GitLab's market share of ~10%, we can comfortably conclude that the vast majority of projects are going to be using GitHub by default. As such, GitHub actions seems the natural successor to Travis for this RFC.

That said, I think it would be reasonable for motivated parties to write an RFC suggesting we build CI config generators for any number of common CI platforms. There is value to making it easy to configure CI even for non-default use-cases, but I think that should be a separate RFC altogether, as that would essentially be an enhancement on the current state of things regardless of whether we switch the default from Travis to GitHub Actions. After all, if we do nothing today, we could keep Travis as the default and build generators for GitHub Actions, CircleCI, Gitlab, or whatever other platforms are commonly used.

Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I completely agree with the goal, I'd like to see this proposal move a slightly different direction:

  • Introduce a new --ci-provider flag to ember new/ember addon
  • Commit to maintaining what we consider the most popular CI providers (I'm thinking TravisCI, GitHub Actions, GitLab CI, and CircleCI)
  • Commit to allowing other CI providers (that are generally useful) to be maintained by the community (essentially "we will review and land if valid, but we do not guarantee that these work)
  • These CI Providers would themselves be their own blueprints with a specific naming scheme (e.g. ci-provider-<provider-name>), this allows addons to ship with a host of additional providers that folks could use (including the ability for organizations to have internal addons with their own custom CI systems built in).
  • Update the interactive ember new RFC (which is not implemented yet), to include an additional prompt which defaults to GH Actions

I think this path provides much better extensibility (and some insulation from this exact issue if GH Actions makes similar changes in the future). Using this strategy combined with ember-cli-update's config/ember-cli-update.json will play very nicely for folks.

@kellyselden - What do you think?

@kellyselden
Copy link
Member Author

Those are good ideas, but I don't think they help us get off Travis today. Travis is essentially unusable today, so I'd prefer shipping GitHub Actions, then work on extracting out blueprints. If we wait for the beautiful end goal, it may never get here.

@rwjblue
Copy link
Member

rwjblue commented Jan 11, 2021

The thing I laid out doesn't have to take longer or delay the timeline at all. I'm mostly talking about how to get to GH Actions by default, not the timing.

I'm happy to send a PR to your fork so we can take a look at it together.

@elwayman02
Copy link
Contributor

I like the idea for a --ci-provider flag as an enhancement, as long as we're still committing to using GitHub Actions as the sane default for when the flag isn't specified.

Ultimately, I agree with @kellyselden that there is a need to get rid of Travis as the default ASAP, and I'm not in favor of debating additional features that could be filed as separate RFCs to further enhance our CI functionality after landing this RFC. I think the changeset required to do what Kelly has proposed is simple, quick, and obvious, and the only thing that needs to be debated for it is whether GitHub Actions are the correct default. If we can all agree on that, then this RFC can move forward and we can continue to discuss the design of a flag, support for multiple CI providers, and so on.

I guess what I'm saying is, look at it this way. Today, we have Travis CI as the default. We could submit RFCs to enhance our CI integration in different ways, of which a --ci-provider flag is one example. Even if we do none of that, however, we still have a default config in our blueprint today. All this RFC needs to cover in scope is whether to change that default (and to which provider); anything else is an unnecessary broadening of scope that should be covered in another RFC, imo.

@rwjblue
Copy link
Member

rwjblue commented Jan 12, 2021

Ultimately, I agree with @kellyselden that there is a need to get rid of Travis as the default ASAP, and I'm not in favor of debating additional features that could be filed as separate RFCs to further enhance our CI functionality after landing this RFC. I think the changeset required to do what Kelly has proposed is simple, quick, and obvious, and the only thing that needs to be debated for it is whether GitHub Actions are the correct default. If we can all agree on that, then this RFC can move forward and we can continue to discuss the design of a flag, support for multiple CI providers, and so on.

I disagree. 😸

I think GitHub Actions is a perfectly reasonable default value, but supporting only a single provider fundamentally sets us up for this exact problem once again (vendor lock in, "favoritism", etc to boot). I'm opposed to supporting only a single provider, and would prefer to remove default CI setup all together (removing the "Travis is broken" issue) than continue the same mistake again.

Sorry. Once bitten, twice shy...

@rwjblue
Copy link
Member

rwjblue commented Jan 12, 2021

Additionally, y'all are making a bit of a false dichotomy here. It isn't a choice of "just swap to GH Actions real quick or we will never land anything". The suggestion I mentioned above could easily be done within the same timeframe (e.g. 3.26'ish, which is the earliest any new RFC could land anyways).

@jelhan
Copy link
Contributor

jelhan commented Jan 12, 2021

Today, we have Travis CI as the default. We could submit RFCs to enhance our CI integration in different ways, of which a --ci-provider flag is one example. Even if we do none of that, however, we still have a default config in our blueprint today. All this RFC needs to cover in scope is whether to change that default (and to which provider); anything else is an unnecessary broadening of scope that should be covered in another RFC, imo.

There is also the alternative to remove TravisCI configuration file from default blueprints without providing an alternative through the default blueprints. This would allow us to experiment with alternative solutions as community packages before.

I think this RFC and the discussion ignores the complexity of how to setup GitHub Actions as CI. It's easy to say that Ember CLI should include it. It's way more difficult to agree on a specific configuration. Let me give some examples for questions that came up when developing create-github-actions-setup-for-ember-addon:

  • Should all jobs run in parallel? CI minutes are free for open source projects on GitHub Actions, you know.
  • Or should they run in stages and fail fast to safe CI minutes? CI minutes are not free for everyone using GitHub Actions. It could get expensive for enterprise customers, you know.
  • And what is about the environment? We may not pay for CI minutes but they have a climate cost for sure.
  • And if we would run them in stages, what stages to use? Lint and primary tests together as we do currently in TravisCI? Or should we run lint as a separate stage before all others?
  • How should we setup the caching? Should we cache node_modules? Or the global cache of the package manager? Or both? What is more performant? In which situations?

Not sure if we could consider all that questions (and many more) as implementation details.

@jelhan
Copy link
Contributor

jelhan commented Jan 12, 2021

I just came around another question. I'm sorry to be that guy who is questioning what seems to be the agreed way forward. But I fear that we are missing important points otherwise.

TravisCI must be explicit activated per-repository additionally to creating the .travis.yml file. Is this the same for GitHub Actions if used in a private repository? Or does GitHub Actions run as soon as the repository contains a workflow? Wondering if there is the risk that it accidentally eats up CI minutes on private projects.

Some enterprise users complained in actions/toolkit#596 that including a CI configuration in the blueprints can be annoying if it does not fit their needs. Especially as it's often recreated by Ember upgrades. This concern may also create costs.

@elwayman02
Copy link
Contributor

elwayman02 commented Jan 12, 2021

TravisCI must be explicit activated per-repository additionally to creating the .travis.yml file.

I don't think this is actually true anymore. I thought Travis switched (at least a year ago) to being able to automatically opt-in every repository as soon as you add the config file?

There is also the alternative to remove TravisCI configuration file from default blueprints without providing an alternative through the default blueprints.

This is strictly a downgrade to what we have now. We should not introduce regressions in what ember-cli currently provides.

I think this RFC and the discussion ignores the complexity of how to setup GitHub Actions as CI. It's easy to say that Ember CLI should include it. It's way more difficult to agree on a specific configuration.

Are these not all questions that we would have to ask for TravisCI (or similar)? As always, we strive to provide a sane default for the majority of projects. People can very easily change their config as needed.

Some enterprise users complained in actions/toolkit#596 that including a CI configuration in the blueprints can be annoying if it does not fit their needs. Especially as it's often recreated by Ember upgrades.

If you are using ember-cli-update for your upgrades (and why would you not at this point?), you should not be seeing files recreated. The only time that would happen is if changes were specifically made to that file between versions, at which point it would show up as a conflict if you've deleted the file. That is trivial to resolve, and if it's a problem of scale we are already working on making ember-cli-update compatible with custom blueprints so that a team can maintain their own blueprint that builds on top of the core Ember blueprints for bootstrapping new projects or upgrading existing ones.

@rwjblue
Copy link
Member

rwjblue commented Jan 12, 2021

TravisCI must be explicit activated per-repository additionally to creating the .travis.yml file.

I don't think this is actually true anymore. I thought Travis switched (at least a year ago) to being able to automatically opt-in every repository as soon as you add the config file?

Ya, this is my understanding also.

@rwjblue
Copy link
Member

rwjblue commented Jan 12, 2021

RE:

Some enterprise users complained in actions/toolkit#596 that including a CI configuration in the blueprints can be annoying if it does not fit their needs. Especially as it's often recreated by Ember upgrades.

My proposal handles that concern nicely (--no-ci-provider or --ci-provider=none), as well as provide a very convenient way for enterprise customers to provide their own CI Provider that works well on their infrastructure (without that "it keeps coming back after every upgrade" problem).

@rwjblue
Copy link
Member

rwjblue commented Jan 12, 2021

I think this RFC and the discussion ignores the complexity of how to setup GitHub Actions as CI. It's easy to say that Ember CLI should include it. It's way more difficult to agree on a specific configuration.

Agree. The RFC should explicitly include the actual configuration file contents.

A blog post could be sufficient explaining this switch.
Existing projects can continue to use Travis CI in its limited capacity.
Those using ember-cli-update could choose to take the new GitHub Actions file or continue using the .travis.yml.
I don't think we cover CI or Travis CI in our docs.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do mention the inclusion of a .travis.yml file here.

@kiwiupover
Copy link

@kellyselden @rwjblue Where are we at with this RFC?

@kellyselden
Copy link
Member Author

In the nicest way possible: I don't have the motivation right now to change/increase the scope of this RFC. I can close it, or someone else can take it on.

@NullVoxPopuli
Copy link
Sponsor Contributor

I can try updating things. ❤️

@elwayman02
Copy link
Contributor

elwayman02 commented Jun 3, 2021

I'm really not certain why this RFC is stalled like this, to be honest. I still don't see the blocking issues as actually blocking, but rather suggested enhancements that should be a separate RFC. If it were so easy to get them done, it wouldn't be 6 months later with this very simple RFC still not merged and implemented. We've spent too much time getting lost in the weeds of what more we could do beyond the scope of this RFC rather than just focusing on solving the immediate problem at hand.

Can we move forward with the RFC as proposed and identify just what is needed to ship it within the defined scope, please? Travis is just completely broken on most community addons still using it, and people are struggling to manually convert their projects without clear guidance on how to use GitHub Actions. We're doing active harm to the community by continuing to ship Travis as the default config.

@NullVoxPopuli
Copy link
Sponsor Contributor

NullVoxPopuli commented Jun 9, 2021

I'm gonna take a stab at suggesting a ci config tomorrow on here.

Agree, @elwayman02. I also don't think we need to select which ci provider. That's scope creep and will bike shed this RFC for eternity 🙃

@jelhan
Copy link
Contributor

jelhan commented Jun 10, 2021

I'm gonna take a stab at suggesting a ci config tomorrow on here.

Agree, @elwayman02. I also don't think we need to select which ci provider. That's scope creep and will bike shed this RFC for eternity 🙃

Are you suggesting to have a high-level CI configuration file, which is than turned by community tools into configuration for specific providers? That would be awesome.

Actually I tried something similar when implementing create-github-actions-setup-for-ember-addon. But it is not that easy if you want to support custom steps (code coverage check, deployment etc.) as well.

@NullVoxPopuli
Copy link
Sponsor Contributor

Are you suggesting to have a high-level CI configuration file, which is than turned by community tools into configuration for specific providers? That would be awesome.

I am not. Just a github-actions config. PR incoming to ember-cli momentarily

@NullVoxPopuli
Copy link
Sponsor Contributor

NullVoxPopuli commented Jun 10, 2021

ember-cli/ember-cli#9569

For Both:

  • split linting and tests in to separate jobs to maximize parallel running to try to best decrease over all CI time
  • support [skip ci] magic commit messages
  • use Volta

For Apps:

  • build once for tests and production
  • cache dependencies between jobs, because GH Action's cache is faster than re-installing (even for small monorepos)

For Addons:

  • run on a weekly cron so that maintained projects can keep an eye on what things break over time (if any)

GitHub Actions also has a limit how much an open source project can use, but it resets every month.
I haven't heard of anyone hitting the limit though.

GitHub Actions doesn't support retrying individual jobs like Travis CI.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actions/runner#432 (comment)

Looks like it is rolling out!

@bertdeblock
Copy link
Member

bertdeblock commented Oct 2, 2021

What should happen with this RFC now that ember-cli/ember-cli#9579 is merged and ember-cli/ember-cli#9644 on the way?

@snewcomer
Copy link
Contributor

I think we can close it!

ember-cli/ember-cli#9579

@snewcomer snewcomer closed this Dec 6, 2021
@snewcomer snewcomer reopened this Dec 6, 2021
@rwjblue rwjblue closed this Dec 17, 2021
@rwjblue
Copy link
Member

rwjblue commented Dec 17, 2021

Ya, seems good to me. Sorry this took so long folks.

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