ditinc / linter-cflint

:shirt: CFLint plugin for Atom Linter
MIT License
4 stars 7 forks source link

CFLint bundling #25

Open KamasamaK opened 7 years ago

KamasamaK commented 7 years ago

What is the reason for having CFLint bundled? I see that Issue #8 is where it started and removed the ability to define the path as configuration. If I understand correctly, Issue #10 explains why it needed to be forked, but if those changes are part of core CFLint, is it still necessary? It seems to me that it's more of a dependency.

That said, the roadmap indicates you are looking to "remove the dependency on Java" so perhaps you are considering a complete rewrite which would justify the lack of dependency on the original project.

For now, it would be great if the path or at least file name were left to be from a configuration file instead of having to modify main.js when upgrading since the included jar is not kept up-to-date.

ryaneberly commented 7 years ago

Ease of use is important. But I don't think you want to tackle "remove dependence on Java". CF itself runs on java -- we should be able to reuse the CF java install without needing to install java somewhere else. that would make sense.

KamasamaK commented 7 years ago

@ryaneberly Since you maintain the CFLint project, these questions would best be directed to you. Do you see the project introducing any breaking changes? If so, would those be reserved for major version changes?

I can see the "ease of use" point, but based on the answers to the questions above I wonder if it would be possible to dynamically pull the latest compatible version then. Either way, my suggestion for storing the location as configuration remains.

ryaneberly commented 7 years ago

I don't see any breaking changes in the near future. the CLI interface and output have stabilized. I think it's far to expect reasonable compatibility on all future 1.x.x releases

KamasamaK commented 7 years ago

Following up with this:

  1. I found node-github-releases which seems to be under atom's organization. There's no documentation, but it looks like it might work to keep CFLint updated if its GitHub releases are kept up-to-date, which it doesn't seem that they are at the moment. I expect the ability to filter by version numbers exists as well.
  2. I also found an NPM CFLint wrapper, which should be easy to integrate, and while it does depend on Java, you would at least be removing the direct dependency since that seems to be one of your goals. It unfortunately doesn't add much at the moment, but if properly maintained I think letting it manage keeping CFLint updated makes more sense than in an editor plugin.