ricardofbarros / linter-js-standard

Atom linter plugin for JavaScript, using JavaScript Standard Style
https://atom.io/packages/linter-js-standard
MIT License
99 stars 48 forks source link

Changes `semistandard` dependency to be a range #72

Closed gabmontes closed 8 years ago

gabmontes commented 8 years ago

Version 7.0.2 of semistandard has some issues like https://github.com/Flet/semistandard/issues/38 and 7.0.3 fixes it. But since the module is required by exact version in package.json, the new version is not pulled from NPM.

This PR changes package.json to require semistandard as a range to include any bug fixes that may become available.

jpstevens commented 8 years ago

Thanks for fixing this @gabmontes. @ricardofbarros can we get this merged please?

despairblue commented 8 years ago

Though you would only get the bugfix when reinstalling the atom package. I don't think that this helps, unless Have you tried to uninstall and install it again? is supposed to be a common question to every bug report :smile:. When everyone installing this package gets different dependencies it rather confuses things.

I'd rather use something like http://greenkeeper.io/ and release on every update of semistandard and standard, but that's just my opinion. In the end it's @ricardofbarros decision.

gabmontes commented 8 years ago

@despairblue you are right as this will not fix current broken installs but every new install will break as semistandard@7.0.2 is buggy. In this scenarios, the advice is to first fix the issue ASAP and then find a better solution if any.

@ricardofbarros this is super quick fix that can be merged immediately. We are 2 weeks since 7.0.3 release and this PR submission. Please comment on this. Thanks!

NickColley commented 8 years ago

:+1: Ran into this issue also, what version should I roll back to?

OlivierCuyp commented 8 years ago

@ricardofbarros are you still maintaining this repository ?

marvinroger commented 8 years ago

@OlivierCuyp his last activity on GitHub was a month ago. I tried to publish a temporary package on Atom with the fix baked in, but there was some kind of conflict, and I am not familiar enough with apm to fix the issue. Can someone try?

ricardofbarros commented 8 years ago

Hey guys.

Yes I'm still maintaining this package and there's a upcoming major version.

I don't want this to be a range because there have been some issues in the past with patch versions of semistandard, standard, etc.. and that's why I block to a specific version that I'm confident it will work.

If I don't block the version it becomes nuisance to debug, for instance someone installs this package and raises an issue that it doesn't work, but I have the latest version and everything is fine, the problem would be the dependencies of this package and that is very painful to coordinate the efforts to debug that specific issue.

marvinroger commented 8 years ago

Fair enough! But in the case of this issue, a range would have fixed things without any interventions from your side. In the meantime, could you please push a fix with semistandard 7.0.4? I keep seeing an error when there is actually none, and it's kind of annoying. ;)

NickColley commented 8 years ago

:+1: for greenkeeper, what version is good to roll back to for now?

NickColley commented 8 years ago

For others with this issue you can roll back to 2.5.0 with apm install linter-js-standard@2.5.0, this release has a ranged dependency.

To 'solve' this issue we could use greenkeeper, however we could also add an option to use the user's globally installed binary?

rankida commented 8 years ago

+1 I ran into this issue as well and it is rather annoying that your linter wrongly tells you something is wrong when it isn't - I am not doing React yet get Definition for rule 'react/jsx-no-duplicate-props' was not found

ricardofbarros commented 8 years ago

Hello guys, @devTristan PR fixed this issue. Many thanks to him. 🎉 🎉

This fix is already published as 3.3.0.

Thanks.