-
Notifications
You must be signed in to change notification settings - Fork 81
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
0.16.1 release #31
Comments
Some help would be appreciated 🙇 I've added you as an owner to the Ruby Gem, thanks for all your hard work! We should probably get together sometime while I'm in Melbourne 😄 |
Hey @mokagio please do that. As long as we get it out and works I am fine |
Hey folks... I was just about to hit the button to push 0.16.1 when a thought occurred... How would be handle bugfixes and other improvements with this versioning system? I mean, we couldn't really release 0.16.2 right, because there's no matching version of SwiftLint (yet). Would we have to wait for a new SwiftLint version to be published? Or would we do something like force updating the ref that RubyGems uses (can it even be done)? I don't have any suggestion in regard, just wanted to get your feedback on 🤔 . |
Hmm, that's a good question. I'm not sure what the best approach is, but I probably wouldn't want to yank/replace the version on Ruby Gems. Not sure what the best approach here is... |
Yep, me either. Would make builds non-deterministic, and just sounds dirty. |
It should be fine of we only follow the major and minor version numbers from swiftlint and leave the patch version free to perform bugfixes or changes not related to swiftlint itself (Danger changes goes here). Given that the patch number means nonbreaking changes Another option is to use the label field to indicate this version is fixing something specifically. Eg 0.16.1-fix132 (major.mjnor.patch-label) but then would be hard to tell which version is the latest if only the label are different Problem is that the mirroring wont be exactly right this way, maybe leave the whole thing even more confusing? Worser would be if Danger implements something awesome that require breaking changes on the plugin system and we are frozen into swiftlint versioning and can't release such update as major version. I am willing to dig little bit further on the issue and came up with more alternatives. I think I borrow the idea from the js community let me check if I can find more info about that. Ultimately if t became a big issue we can have our own versioning scheme and make sure underling swiftlint version is easily found (maybe part of the gem description) :) |
I don't know exactly how "native extensions" work, but what are the advantages over installing swiftlint when the plugin is called? This way the SwiftLint version to be installed could be a parameter so we don't need to release a new version when SwiftLint does. |
@marcelofabri native extensions works like a post-install hook where you can compile code dependencies that need to be compiled on final user machine, usually this is used for C libraries which you can't ship as binary given that it would only work with the environment were de binary was generated from therefore you need to compile after install to ensure it will work with the final user machine. The code is leveraging this installation phase to download the portable swiftlint version that will be used at runtime. The idea I had when I implemented the managed swiftlint installation is that I would like it to be predictable, meaning you would know exactly what version of swiflint you would be using at any given version, this way 1) as user I can choose which one is better for me, perhaps I am not ready for the latest version so I will stick with this other version and 2) as maintainer I know exactly what are the quirks of swiflint I need to work around in order to provide the best UX. Eg few versions ago swiftlint had a problem to do selective linting and we endup making a workaround here in order to make it work as we wanted, but then the swiftlint folks fixed the problem and we were left with the workaround which weren't necessary anymore, how to drop our code and ensure it will work regardless the swiftlint version, as a result I thought about mirroring they version so we can be free to implement and drop any workaround/features as we need to. Hope that give enough context and reasoning behind matching both versions =) I think it is nice to have this conversation because as you said we might want to leave swiflint version unmanaged and that might be the best choice after all. Thanks for sharing your thoughts |
Love this constructive chats too! 💖 I think we've put a lot of stuff on the plate, and we can group it in two buckets:
These are my two cents: I think we should stick with our own semantic versioning, without mirroring SwiftLint's. This will give us the flexibility to release fixes and new features while still supporting the same version of SwiftLint. In this scenario we'd have to find a way to state which version of the tool the gem uses. Not sure what's the best way to do it, maybe an exec command?
I think the consistency that using SemVer would give us outweighs the convenience of knowing which SwiftLint version the plugin expect just by looking at his version. Here's an example: we'll reach a point where we'll be happy with the state of the plugin and we'd like to call it "stable". The best way to communicate that would be by releasing a Something else that just popped in my mind: Bundler uses SemVer to resolve the dependency graph. Without adopting SemVer for the context of the plugin behaviour we might prevent users to use the Regarding whether or not to manage SwiftLint, I think we could do both 😄 . We could simply add a config option to the plugin to specify the path to the SwiftLint binary to use. The default behaviour which should suit most users would be to just use the one build by the gem. Those users that want to use a different version for whatever reason could set the path to their binary. # Dangerfile
swiftlint.bin_path = '/usr/local/bin/swiftlint' |
Hey @mokagio I am ok as well with going without mirroring the version with SwiftLint and create something that makes it easy to query the tool version as you mentioned. Regarding managing the swiftlint tool my only concern of not managing it is the workarounds we have on the codebase in order to bypass swiftlint limitations. Eg few versions ago we had the "selective linting" problem with swiftlint which we patched here on the plugin in order to provide that feature, knowing the underline tool help us to know what is needed and what is not or even predict that the plugin will work. Once we open the gate to select the bin_path it can eventually hit some integration problems. But if we believe that was a one off case and shouldn't happen again I have no obstruction to opening the config field and let the user specify whatever bin he wants to use =) |
@thiagofelix good point on the risks of using a version of SwiftLint that the gem is not expecting... 🤔. I guess we have to options:
I think leaving it locked might make the tool slightly less flexible, but definitely safer. |
@mokagio I think we can tell upfront in documentstio which version the plugin was designed for and warn the user about the bin_path property to use at own risk kind of thing. |
I'll try to address this during before the end of the weekend, been a bit busy with other things. |
Don't worry about it – take care of yourself first @mokagio 😄 |
Also @mokagio I think we can release those features separately from the swiftlint new packaging implementation |
* Add CLI returning versions We have two commands: - version, the plugin version - swiftlint_version, the version of the SwiftLint binary used by the plugin * Set plugin version to 0.4.0 See discussion in https://github.com/ashfurrow/danger-swiftlint/issues/31
Just released 0.4.0. Thanks @ashfurrow and @thiagofelix for the feedback and the constructive discussion 😄 . |
Thanks @mokagio for taking it further and releasing the new version. |
@thiagofelix now that https://github.com/ashfurrow/danger-swiftlint/pull/28 has been merged, how do you feel about releasing a new version to start using the new system? /cc @ashfurrow
😄🚢
If you're busy or need help I'm happy to take care of it, but as the author of this great change you deserve to push the button 👍 .
The text was updated successfully, but these errors were encountered: