-
Notifications
You must be signed in to change notification settings - Fork 243
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
Tweak rockspec to normalize fields, callout spec version, etc. #357
Conversation
455c402
to
cf1247a
Compare
80bfe55
to
ba36a08
Compare
ba36a08
to
b383a19
Compare
Based on comments on an obsolete commit here I have rebased this PR to simplify a bit and just re-versioned the rockspec to |
Initially I thought we should hold off on this until just before a release, but I actually don't see why it matters at all. This is the dev/scm/vcs rockspec, there is no reason to sync it to the stable release cycle. |
Just for the record, the urgency on this (not to wait for a release) is that it's holding up other dependent work such as migrating the test suite, which I want to be able to run with |
@@ -1,5 +1,6 @@ | |||
rockspec_format = "3.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is ok for the DEV one, not sure about the release ones, since older luarocks installs will not be able to install Penlight anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay. Are you sure that's actually how it works? I thought some fields were just ignored if it doesn't understand them. If we have to strip out the test stuff (see #358) for the release rockspecs I guess that's workable, but it should still be in the dev one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error: Rockspec format 3.0 is not supported, please upgrade LuaRocks.
Kong version: 0.13.0
[Kong-0.13.0:/kong]# luarocks --version
/usr/local/bin/luarocks 2.4.3
LuaRocks main command-line interface
[Kong-0.13.0:/kong]#
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the upshot is that for best compatibility we need to release stable versions using an older rockspec version ... which means dropping a couple of fields such as the testing definitions when the rockspec gets copied to stable tagged versions. If I'm understanding that right that shouldn't be too much of a problem, but it might be nice to automate that (or at least write a release checklist so we do it consistently).
Is there something I'm missing? Not having test runner information in the rockspec doesn't seem like a problem for stable release rockspecs, and personally I don't have a whole lot of sympathy for people trying to install bleeding edge Git HEAD software using very old tooling (2.4.3 is over 3 years and 15 releases old).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrt
write a release checklist so we do it consistently
see https://github.com/lunarmodules/Penlight/blob/master/CONTRIBUTING.md#release-instructions-for-a-new-version which is referred to from the changelog as well
Only the last commit here is relevant, all the others are part of #355.This is on hold until...
[ ] Immediately before a tagged release so the SCM rockspec rename doesn't cause issues between releases