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 Elm language support #934

Merged
merged 67 commits into from
Nov 27, 2018
Merged

Add Elm language support #934

merged 67 commits into from
Nov 27, 2018

Conversation

rtfeldman
Copy link
Contributor

This adds Elm support to Travis.

Happy to do as much revision to this as necessary! Also I'd be happy to push branches of some of my Elm projects to have them use language: elm in their .travis.yml files for the staging tests.

@BanzaiMan
Copy link
Contributor

Hi, there! Thank you for your interest in adding Elm to Travis CI.

If you have not done so, please read https://docs.travis-ci.com/user/languages/community-supported-languages/ for some additional details. (Some of the details are a bit out of date; in particular, the bits on travis-core in https://docs.travis-ci.com/user/languages/community-supported-languages/#Code since that library is no longer relevant to community-supported languages. See travis-ci/docs-travis-ci-com#819.)

@BanzaiMan BanzaiMan self-assigned this Jan 20, 2017
@rtfeldman
Copy link
Contributor Author

Yep! That page was my starting point. 😁

@rtfeldman
Copy link
Contributor Author

@BanzaiMan Just wanted to check - is there anything you need from me on this? Or just waiting on review?

@BanzaiMan
Copy link
Contributor

Please gather a few other volunteers to provide support. Thanks.

@lukewestby
Copy link

@BanzaiMan @rtfeldman I can help

@avh4
Copy link

avh4 commented Feb 6, 2017

I can help as well. I'll also add this to the elm-community projects list so we can document who's actively part of the support team.

@rtfeldman
Copy link
Contributor Author

@BanzaiMan done! 😄

@stoeffel
Copy link

stoeffel commented Feb 6, 2017

@rtfeldman I can help!

@rtfeldman
Copy link
Contributor Author

@BanzaiMan friendly ping! 😃

@rtfeldman
Copy link
Contributor Author

@BanzaiMan anything we still need to do to unblock review?

@rtfeldman
Copy link
Contributor Author

@BanzaiMan friendly ping! 🙂

end

def install_elm
sh.cmd 'npm install -g [email protected]', retry: true

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@BanzaiMan
Copy link
Contributor

Am I correct to understand that Elm requires a Node.js/JavaScript image?

@rtfeldman
Copy link
Contributor Author

Am I correct to understand that Elm requires a Node.js/JavaScript image?

@BanzaiMan Elm doesn't, but elm-test does, and it's reasonable to assume people will want that for CI. 😄

@BanzaiMan
Copy link
Contributor

Are there other requirements, such as required Node.js version and NPM version?

@rtfeldman
Copy link
Contributor Author

@BanzaiMan ideally Node 6 and npm 3 (any minor versions within those major versions should be fine).

module Travis
module Build
class Script
class Elm < Script

This comment was marked as spam.

This comment was marked as spam.

.nvmrc Outdated
@@ -0,0 +1 @@
6.10.0

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

module Travis
module Build
class Script
class Elm < Travis::Build::Script::NodeJs

This comment was marked as spam.

end

def install_elm
npm_install "-g elm@elm-#{elm_version}"

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

Copy link
Contributor

@BanzaiMan BanzaiMan left a comment

Choose a reason for hiding this comment

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

With the suggested changes, the build proceeds to user-defined phases.

end

def install_elm
npm_install "-g elm@elm-#{elm_version}"

This comment was marked as spam.

lib/travis/build/script/elm.rb Outdated Show resolved Hide resolved
DEFAULTS = {
elm: '0.19.0',
elm_test: '0.19.0',
elm_format: '0.19.0'

This comment was marked as spam.

This comment was marked as spam.

Copy link
Contributor

@BanzaiMan BanzaiMan left a comment

Choose a reason for hiding this comment

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

More suggestions; for improving log readability.

lib/travis/build/script/elm.rb Show resolved Hide resolved
lib/travis/build/script/elm.rb Show resolved Hide resolved
lib/travis/build/script/elm.rb Show resolved Hide resolved
BanzaiMan added a commit that referenced this pull request Nov 20, 2018
@BanzaiMan
Copy link
Contributor

@BanzaiMan
Copy link
Contributor

BanzaiMan commented Nov 20, 2018

Thank you, @rtfeldman! It looks very good.

We'll be moving to https://travis-ci.community/ for support issues. We'll tweak the display text accordingly before releasing.

The current plan is to announce Elm support on December 4 November 27, 2018.

BanzaiMan added a commit that referenced this pull request Nov 20, 2018
@BanzaiMan
Copy link
Contributor

Needs docs PR.

@rtfeldman
Copy link
Contributor Author

Needs docs PR.

Is this something I should do? 😄

@BanzaiMan
Copy link
Contributor

@rtfeldman I think I can do this, given the short time frame we've got. I should've pointed this out much earlier.

@BanzaiMan
Copy link
Contributor

@rtfeldman What is the intended interactions among elm, elm_test, and elm_format? Currently, these can be independently defined, which I think is OK, but if one changes only elm, then elm_test and elm_format fall out of sync with it. That is to say:

language: elm
elm: 0.20.0 # a future release

will use Elm 0.20.0, but elm_test and elm_format remains at elm0.19.0, which is the default.

I am unfamiliar with the Elm ecosystem, but I imagine that this is not an ideal situation. From the user's perspective, its seems to me that elm value should propagate to the other two tools that depend on it (unless, of course, elm_test or elm_format is overridden.)

@BanzaiMan
Copy link
Contributor

On a related note: how useful is the matrix expansion on elm_test and elm_format? It makes sense for the repository to test on multiple Elm versions, but I would like to not have to expand on elm_test and elm_format, to prevent enormous build matrices.

@BanzaiMan
Copy link
Contributor

@rtfeldman

We plan to announce this tomorrow (2018-11-27) with the changes I suggested earlier. Namely:

  1. The elm version propagates to elm_test and elm_format tag, unless these are explicitly overridden in the configuration.
  2. There will be no matrix expansion on elm_test and elm_format.

@rtfeldman
Copy link
Contributor Author

@BanzaiMan that all sounds good!

Do you need me to make those changes?

@BanzaiMan
Copy link
Contributor

@rtfeldman I can work on the changes I've suggested.

@BanzaiMan
Copy link
Contributor

BanzaiMan commented Nov 26, 2018

See 9fbe7e4, including specs, which show that:

  1. The elm value drives elm_test and elm_format versions.
  2. The elm value could be with or without the elm prefix (e.g., 0.19.0 or elm0.19.0) and TRAVIS_ELM_VERSION will retain it if the default is used, or the user supplies it; but TRAVIS_ELM_TEST_VERSION and TRAVIS_ELM_FORMAT_VERSION will always be prefixed with elm.

@rtfeldman
Copy link
Contributor Author

Looks good, thanks for all your help @BanzaiMan! Super excited to see this land! 😃

@BanzaiMan BanzaiMan merged commit 7070ea2 into travis-ci:master Nov 27, 2018
@rtfeldman rtfeldman deleted the add-elm branch November 27, 2018 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.