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

Do not hardcode specific parser dep as dependency #410

Closed
mbj opened this issue Jul 31, 2013 · 16 comments
Closed

Do not hardcode specific parser dep as dependency #410

mbj opened this issue Jul 31, 2013 · 16 comments

Comments

@mbj
Copy link
Contributor

mbj commented Jul 31, 2013

Rubocop is not the only consumer of parser, my mutant and unparser projects also require parser. For each parser release we have a breakage period where the gems in the rom project cannot bundle ;)

I'd love to see a dependency like ~> 2.0.0.pre5 in your dependecies, so you dont break other peoples dependency tree.

@dkubb
Copy link
Contributor

dkubb commented Jul 31, 2013

Another approach that I've used in cases where I really trust a dependency is to specify something like:

 s.add_runtime_dependency('parser', '~> 2.0', '>= 2.0.0.pre5')

This will allow anything in the 2.x series, while also asserting the minimum version be at least 2.0.0.pre5 or greater.

So far parser seems really rock-solid, so I have no reason to believe that it won't follow semver so this should work well while also eliminating most types of version conflicts.

@bbatsov
Copy link
Collaborator

bbatsov commented Jul 31, 2013

The problem is that most of Parser's releases break RuboCop because it uses absolutely every APi that Parser exposes and most prereleases change some of them. If we allow newer parser 2.0 prereleases the users will probably often run into trouble when doing gem updates. That problem will go away once 2.0 final is released and the API is frozen.

@mbj
Copy link
Contributor Author

mbj commented Jul 31, 2013

@bbatsov IMHO having a development gem that bundles, but might fail is less a thread than breaking an entire dependency resolution. Pls semver your dependencies :D.

@solnic
Copy link

solnic commented Aug 1, 2013

ugh +1 - right now all my builds fail because of the version conflict

@edzhelyov
Copy link
Collaborator

I understand your problem guys, but I think it's not correct to have rubocop or any other tool in the Gemfile.
I haven't thought about a solution, but it bothers me when I see such a tool listed in the Gemfile. It should not be installed with the dependency management for the rest of the system.

@solnic
Copy link

solnic commented Aug 1, 2013

@edzhelyov ugh, why? we use rubocop as our development dependency and we share that dependency across a whole bunch of libraries. Even if it was not in the Gemfile we'd still get a version conflict because of the pinned version for parser gem. So, if you don't fix it, we can't use it.

@edzhelyov
Copy link
Collaborator

Tool like RuboCop only facilitates the development, but I don't think it's a development dependency. Running the tool and its return code maybe a part of the build, but what will happen if you require a tool not written in ruby ?

I don't like it when I see it in our projects too. What will happen if I have a tool that depends on parser 1.0 and works flawlessly and then another tool that depends on parser 2.0 and still works. I don't think that the installation and usage of these tools should force particular version on them.

I think we incorrectly use development dependencies for such tools and I wanted to bring the issue in a public discussion.

As for the parser version, I'll wait for @bbatsov it's his call.

@mbj
Copy link
Contributor Author

mbj commented Aug 1, 2013

@edzhelyov Decision when to use a specific tool should be up to the user, not the developer of the tool.

@edzhelyov
Copy link
Collaborator

@mbj I think there is some misunderstanding here.

I say that tools like RuboCop, that are run as standalone binaries, should not be installed following the dependency tree of the code. For me, it's the same as we use git for development and I bundle the git dependencies with my own dependencies.

Of course, this is my opinion. Which as it seems you don't share.

@solnic
Copy link

solnic commented Aug 1, 2013

@edzhelyov it's a nice theory however it's not gonna work in our case - we distribute our development dependencies across a lot of projects as we have unified development environment that we want to control in one place. This place is our shared development Gemfile. If we wanted to decouple those development deps then we wouldn't be able to control them which would lead to problems like something passes for one person and fails for another person because they don't use the same version of tool X.

You can fix the problem just like @dkubb suggested, if there's going to be a breakage when new parser pre-release is available we will deal with that. There's a chance it'll all work from now on too. Whatever happens it won't be as bad as it is now where all our builds fail because of the version conflict.

@yujinakayama
Copy link
Collaborator

How about this solution:

  • Until official release of Parser 2.0.0, we release RuboCop as both stable and development edition for each version (e.g. 0.10.1 and 0.10.1.dev)
  • The stable editions have fixed Parser dependency like 2.0.0.pre5 (same as the current)
  • The development editions have loose Parser dependency like ~> 2.0.0.pre5
  • People who use other Parser consumer gems and are willing to accept the breakage risk with Parser's new prerelease, can use development edition
  • Other people use stable edition (gem install rubocop won't install development edition unless --pre option is specified)

I guess most users don't know that Parser is in prerelease term currently, and I don't want them to troubleshoot. From my experience with the past Parser's prereleases, sometimes we took several days to adapt RuboCop to the new version of Parser. I think we cannot accept exposing broken RuboCop such a long time. So, currently I think this solution is a workable compromise.

@mbj
Copy link
Contributor Author

mbj commented Aug 1, 2013

@yujinakayama +1

@edzhelyov
Copy link
Collaborator

@yujinakayama +1. Very nice approach

@bbatsov
Copy link
Collaborator

bbatsov commented Aug 6, 2013

Btw, according to @whitequark's original plan whitequark/parser#51 Parser 2.0.0 should be released fairly soon I guess.

@bbatsov bbatsov closed this as completed in 5f85c9f Aug 7, 2013
@bbatsov
Copy link
Collaborator

bbatsov commented Aug 7, 2013

@whitequark commented that there will be just one bugfix before Parser 2.0.0 is released, so I think it's safe to set the dependency as suggested by @dkubb:

s.add_runtime_dependency('parser', '~> 2.0', '>= 2.0.0.pre5')

Doing an extra development release seems like an overkill at this point.

@mbj
Copy link
Contributor Author

mbj commented Aug 7, 2013

@bbatsov Thx!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants