search

varvet / pundit

Minimal authorization through OO design and pure Ruby classes
MIT License
8.24k stars 627 forks source link

Add Truffleruby head to CI #662

Closed gogainda closed 3 years ago

gogainda commented 3 years ago

@Linuus pls review/merge

dgmstuart commented 3 years ago

@gogainda Hi - I'm not familiar with Truffleruby. If we introduce it to the build then we're to some extent committing to supporting it, and I'm not sure there's a good reason for us to do so? Let me know what you think.

gogainda commented 3 years ago

Truffleruby is intended to be compatible with MRI ruby 2.7 . If you encounter any issues where Truffleruby build fails and MRI ruby 2.7 doesn't it means that it's up to Truffleruby team to take care of it in here https://github.com/oracle/truffleruby/issues

dgmstuart commented 3 years ago

@gogainda the issue is that if a PR fails when building against Truffleruby, but passes for all other rubies, then that fails the build overall, and we're unlikely to be able to do anything about it ourselves.

My understanding is that the reason for building against multiple ruby versions is so that we can fix any issues in the Pundit source code if they happen to not be compatible with a particular ruby version. But it sounds like if we had a failure against Truffleruby, then it would be the Truffleruby team who would have to resolve it? I don't see why it's worthwhile for us to make this project dependent on that team.

@olleolleolle do you have a take on this?

olleolleolle commented 3 years ago

@dgmstuart My simplest take: Add to the matrix!

Implementations of Ruby which break could if they cause issues that are not met with action from the language implementer/champion, be put in the allow_failures section.

Perspective: I have added lots of JRuby to CI matrixes, in the hunt for bugs in the JRuby implementation. Many library authors are willing to have JRuby in their matrix, to help with this compatibility scavenger hunt.

dgmstuart commented 3 years ago

Closed in favour of https://github.com/varvet/pundit/pull/678 which adds it to allow_failures - thanks @olleolleolle