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

Add org-wunderlist recipe #3000

Merged
merged 1 commit into from Aug 18, 2015

Conversation

Projects
None yet
2 participants
@myuhe
Copy link
Contributor

myuhe commented Aug 8, 2015

No description provided.

@myuhe

This comment has been minimized.

Copy link
Contributor Author

myuhe commented Aug 13, 2015

ping

@purcell

This comment has been minimized.

Copy link
Member

purcell commented Aug 14, 2015

Thanks, sorry -- I missed this because I had a busy weekend!

The symbol prefix here should really be org-wunderlist-, not org-wlist-, according to the elisp coding conventions. I also think that prefix would be clearer. What do you think?

@myuhe

This comment has been minimized.

Copy link
Contributor Author

myuhe commented Aug 15, 2015

Thank you for your response.
prefix org-wunderlist is too long. prefix should be shorter.
For instance yasnippet uses prefix yas- . I think it is appropriate.

https://github.com/capitaomorte/yasnippet

@purcell

This comment has been minimized.

Copy link
Member

purcell commented Aug 15, 2015

yas is much too short, especially compared to yasnippet, so they're doing it wrong too, and setting a bad example. The conventions are there to help the community, and they only say that the prefix and filename should match: so yas would be okay if the package were called yas.el, and the org-wlist- prefix suggests that your package should be called org-wlist.el.

I'm not going to argue, but since you're the author of several packages I'd encourage you to set a good example by following the conventions. (A compromise would be to just use org-wunderlist- for the public symbols: there aren't many.)

Just let me know what you prefer.

@purcell

This comment has been minimized.

Copy link
Member

purcell commented Aug 15, 2015

Note that there are some really badly-behaved packages out there like multiple-cursors and its awful mc- and mc/ prefixes, so I realise I'm being picky here. :-)

@myuhe

This comment has been minimized.

Copy link
Contributor Author

myuhe commented Aug 18, 2015

Thanks.
I renamed wlist to wunderlist. myuhe/org-wunderlist.el@58e3695

check it.

purcell added a commit that referenced this pull request Aug 18, 2015

Merge pull request #3000 from myuhe/org-wunderlist
Add org-wunderlist recipe

@purcell purcell merged commit 44019e5 into melpa:master Aug 18, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@purcell

This comment has been minimized.

Copy link
Member

purcell commented Aug 18, 2015

Awesome - thanks for your patience. I really think the consistency is worth the longer identifiers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment