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

merge in flymake-puppet? #116

Closed
anarcat opened this issue Mar 6, 2019 · 8 comments
Closed

merge in flymake-puppet? #116

anarcat opened this issue Mar 6, 2019 · 8 comments

Comments

@anarcat
Copy link

anarcat commented Mar 6, 2019

would it be a good idea to merge flymake-puppet into this repository?

https://github.com/grimradical/puppet-flymake

It's 35 lines of code to enable flymake integration... seems like it would be a great improvement at little cost, especially since it doesn't seem to be maintained on its own.

Thanks for your work!

@anarcat
Copy link
Author

anarcat commented Mar 6, 2019

I've opened issue grimradical/puppet-flymake#2 on the other side to see if they're interested...

@bastelfreak
Copy link
Member

Hi @anarcat. I would be fine with merging it, but I don't know enough the project and emacs to make this properly. Can you propose a PR for this?

@anarcat
Copy link
Author

anarcat commented Mar 10, 2019

i think it's just a matter of copying the file over, but i must admit i'm not sure myself.

@sten0
Copy link
Contributor

sten0 commented Mar 11, 2019

I took a quick look at puppet-flymake and noticed one possible blocker and one possible TODO. tldr; skip to the end.

The possible blocker: puppet-flymake doesn't have a copyright or license statement (eg: author retains ). That said, it asserts it "is mostly based on flymake-coffee" (GPL-3+). If the number of changes is small enough to not be significant for the purposes of copyright, then the file can be copied and pasted, Deepak retained as an author (in the headers) but not as a copyright holder.

The possible TODO: the puppet-mode-hook design might need to be changed.

I found this substantially similar (and newer) project, also based on flymake-coffee: https://github.com/benprew/flymake-puppet. It declares GPL-3+, but also doesn't credit flymake-coffee authors in its copyright statement. Flymake-puppet is also on MELPA unstable (https://melpa.org/#/flymake-puppet), and has a decent number of users (59th percentile), so I feel like it might be a better source to merge. That said, it's now abandoneware "Deprecated in favor of: flycheck-puppet".

@anarcat, how do you feel about using flycheck? ISTM that flycheck+user config could solve this.
@bastelfreak, en lieu of putting the burden on the user, puppet-mode can have an optional dependency on flycheck is flycheck is found, and echo something like "Please install flycheck for enhanced syntax checking" to the minibuffer. @anarcat, in this case I'd add a Recommends flycheck to the Debian package and everything work Just Work™. @bastelfreak, of course if you'd like to push all users to use flycheck you can use a hard requires, but I like the maximal user freedom of the former approach.
https://www.masteringemacs.org/article/spotlight-flycheck-a-flymake-replacement
http://www.flycheck.org/en/latest/user/flycheck-versus-flymake.html

@sten0
Copy link
Contributor

sten0 commented Mar 11, 2019

Another option, for the sake of completeness: Use https://github.com/purcell/flymake-easy to generate the flymake bits and integrate, thus reducing the number of copyright holders, which means fewer puppet-mode [copyright holders] would need to be contacted for FSF copyright assignment, if inclusion into upstream GNU Emacs is ever desired.

@anarcat
Copy link
Author

anarcat commented Mar 11, 2019 via email

@anarcat
Copy link
Author

anarcat commented Mar 11, 2019

Quick update: I just installed flycheck, and it seems to support puppet manifests out of the box.

So forget about flymake-puppet altogether - it's not necessary anymore. I'm closing this ticket. :)

Sorry for the noise!

@anarcat anarcat closed this as completed Mar 11, 2019
@rski
Copy link
Member

rski commented Mar 11, 2019

I'm a bit late to this party, but here's my two cents;
a) emacs packages tend to avoid opinionated decisions like this if possible, and trust me I do have strong opinions on flymake vs flycheck ;)
b) Generally when things like this are tied together (e.g. eglot + flymake), it's the choice of the author because they use the $otherthing heavily. I don't think this is the case with flymake and puppet-mode.
c) The extra convenience from putting both in the same repo is not that much imo. If there was strong reasons to merge them, I'd at the very minimum ask that puppet-mode and flymake-puppet are still separate, so after you install the package from melpa, you'd still have to do two requires. Then the flycheck people can still use it without having flymake interfere, e.g. affect startup times. That's not a big step away from two different packages anyway, It'd be one line in my init.el.
d) Flymake is in an interesting state of flux at the moment. I don't think there is strong willingness within voxpupuli to maintain a flymake library that would support both( maybe?) Or would require changes to work with the new api in the future.
I'm happy that flycheck works for you, it generally tends to work out of the box most of the time.

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

4 participants