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.

Sign up for GitHub

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

Jump to bottom

Add Truffleruby head to CI #2099

Merged
merged 9 commits into from
Sep 1, 2020
Merged

Add Truffleruby head to CI #2099

merged 9 commits into from
Sep 1, 2020

Conversation

gogainda
Copy link
Contributor

No description provided.

@gogainda
Copy link
Contributor Author

oracle/truffleruby#2084

@dblock
Copy link
Member

dblock commented Aug 31, 2020

I am open to including this into the set of builds that allow failures if someone (you?) wants to help out with truffleruby-related issues. We haven't had much success in the past with anyone caring about things like rbx.

@gogainda
Copy link
Contributor Author

@eregon I filled out an issue , but I am not sure when it's going to be fixed. Since tests itself are passing prb I can mark it as allow failures for now

@eregon
Copy link
Contributor

eregon commented Aug 31, 2020 •
edited
Loading

One potential workaround is to run RuboCop only on MRI, it seems redundant to run it on TruffleRuby anyway.
This could be done by changing the Rakefile:

grape/Rakefile

Line 27 in 7e95ac8

task default: %i[rubocop spec]

Otherwise the error should be solved by rubocop/rubocop#8602, and then we need to wait the next RuboCop release and make sure it's used by this repo.

@dblock
Copy link
Member

dblock commented Aug 31, 2020

I would take a change to run RuboCop once for the entire suite.

@eregon eregon mentioned this pull request Aug 31, 2020
Merged
8 tasks
@eregon
Copy link
Contributor

eregon commented Aug 31, 2020

OK, then the easiest way to do that is to remove it as a dependency of the Rakefile default task, and to add an extra script: rubocop in TravisCI.
@gogainda Can you try that? Otherwise I can give it a shot.

@gogainda
Copy link
Contributor Author

@eregon yep, will do this

@gogainda
Copy link
Contributor Author

@dblock now I run rubocop once and truffleruby tests are green

@gogainda
Copy link
Contributor Author

gogainda commented Sep 1, 2020

@dblock @eregon the issue with rubocop is fixed in 0.90 but I think we should leave upgrading rubocop version outside of this PR

@dblock
Copy link
Member

dblock commented Sep 1, 2020

  1. Let's leave Rakefile alone and run rake spec in Travis instead, so that developers don't forget that Rubocop offenses need to be fixed during development.
  2. I'd like us to add truffle-ruby to allow_failures even though it's not failing, as I don't personally have the bandwidth to support multiple runtimes, similarly to rbx, and others.

@gogainda
Copy link
Contributor Author

gogainda commented Sep 1, 2020

@dblock sure, but I am not really familiar with travis yml syntax, if you have an example it would be great. Regarding allow_failures - I can add it

dblock
dblock reviewed Sep 1, 2020
View reviewed changes
.travis.yml Outdated
@@ -11,6 +11,9 @@ matrix:
- rvm: 2.7.1
script:
- bundle exec danger
- rvm: 2.7.1
script:
- rubocop
Copy link
Member

Choose a reason for hiding this comment

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

This should be bundle exec rubocop to pin to the version in Gemfile.

@dblock
Copy link
Member

dblock commented Sep 1, 2020 •
edited
Loading

@dblock sure, but I am not really familiar with travis yml syntax, if you have an example it would be great. Regarding allow_failures - I can add it

script: bundle exec rake spec

I think you can do it once at top level and the other script: ... will override it as needed for danger and rubocop, but please check in Travis that it does call the right things.

@gogainda gogainda changed the title Update .travis.yml Add Truffleruby head to CI Sep 1, 2020
@gogainda
Copy link
Contributor Author

gogainda commented Sep 1, 2020

@dblock addressed your comments

dblock
dblock requested changes Sep 1, 2020
View reviewed changes
Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

Add to CHANGELOG "Added truffleruby to Travis-CI"

.travis.yml Outdated
@@ -35,12 +34,12 @@ matrix:
- rvm: 2.7.1
gemfile: gemfiles/multi_json.gemfile
script:
- bundle exec rake
- bundle exec rubocop
Copy link
Member

Choose a reason for hiding this comment

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

This line should be removed and default to rake spec, we're running all the usual tests, not rubocop.

.travis.yml Outdated
- bundle exec rspec spec/integration/multi_json
- rvm: 2.7.1
gemfile: gemfiles/multi_xml.gemfile
script:
- bundle exec rake
- bundle exec rubocop
Copy link
Member

Choose a reason for hiding this comment

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

same

@gogainda
Copy link
Contributor Author

gogainda commented Sep 1, 2020

@dblock done

@dblock dblock merged commit d40d697 into ruby-grape:master Sep 1, 2020
@dblock
Copy link
Member

dblock commented Sep 1, 2020

Merged, thanks for this.

@gogainda
Copy link
Contributor Author

gogainda commented Sep 1, 2020

@eregon

@anakinj anakinj mentioned this pull request Dec 26, 2020
Merged
This was referenced Mar 8, 2021
Closed
Closed
Closed
Closed
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dblock dblock dblock requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gogainda @dblock @eregon