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

Add recipe for org-wiki. #4268

Closed
wants to merge 1 commit into from
Closed

Add recipe for org-wiki. #4268

wants to merge 1 commit into from

Conversation

caiorss
Copy link

@caiorss caiorss commented Sep 24, 2016

Brief summary of what the package does

Org-wiki is an org-mode extension that provides tools to manage and build a desktop wiki where each wiki page is an org-mode file.

Direct link to the package repository

https://github.com/caiorss/org-wiki

Your association with the package

Author and maintainer.

Relevant communications with the upstream package maintainer

None

Checklist

  • [ x] I've read CONTRIBUTING.md
  • [ x] I've built and installed the package using the instructions in the README

@syohex syohex added recipes awaiting-upstream Awaiting action from an upstream maintainer labels Sep 24, 2016
@syohex
Copy link
Contributor

syohex commented Sep 24, 2016

I have sent PR(caiorss/org-wiki#6) about package header.

And please check following comment

  • There are many byte-compile warnings
    • Function directory-list is not defined, this causes run-time error
  • no autoload cookie
  • Please use - separator(-- can be used for private functions/variables) instead of / for emacs lisp coding convention

@caiorss
Copy link
Author

caiorss commented Sep 24, 2016

Why not use the slash separator (/) instead of dash (-) because it works like a Clojure namespaces that uses / syntax ?

It is easier in this way to group the functions by common prefix separating the prefix of the function and grouping them by pseudo "namespaces" that is more completion friendly and also better to the eyes. Why not innovate ? The slash convention doesn't mess anything on Emacs.

By using the slash it is more easier to know to the package the function belongs.

caiorss added a commit to caiorss/org-wiki that referenced this pull request Sep 24, 2016
 - Issue: Function directory-list is not defined, this causes run-time
   error. (melpa/melpa#4268 (comment))
caiorss added a commit to caiorss/org-wiki that referenced this pull request Sep 24, 2016
@syohex
Copy link
Contributor

syohex commented Sep 25, 2016

Why not use the slash separator (/) instead of dash (-) because it works like a Clojure namespaces that uses / syntax ?

It is Emacs Lisp coding convention.

https://www.gnu.org/software/emacs/manual/html_node/elisp/Coding-Conventions.html

caiorss added a commit to caiorss/org-wiki that referenced this pull request Sep 25, 2016
@caiorss
Copy link
Author

caiorss commented Sep 25, 2016

All right, I have updated the code according to the Elisp coding conventions.

But this conventions should be updated. Emacs lacks namespaces and the user has a hard time to spot visually to what package the function belongs. This notation taken from Clojure separates better the prefix of package name from the function name and it makes the functions more search friendly and completion friendly.

caiorss added a commit to caiorss/org-wiki that referenced this pull request Sep 25, 2016
@purcell
Copy link
Member

purcell commented Sep 27, 2016

But this conventions should be updated. Emacs lacks namespaces and the user has a hard time to spot visually to what package the function belongs. This notation taken from Clojure separates better the prefix of package name from the function name and it makes the functions more search friendly and completion friendly.

We're not here to discuss language design, sorry, but you can bring it up on the emacs mailing list if you feel strongly.

Whatever you think of them currently, the elisp conventions are there to support uniformity of code, so that we can all comfortably read and contribute to each other's code, so this is a matter of community benefit rather than individual preferences. (And I say this as someone who has worked a lot with Clojure, so I agree that its namespaces are nice.)

Some additional specific feedback:

  • Don't indent subsequent docstring lines: Example: etc. should be Example:.
  • M-x checkdoc will help you tidy up docstrings. And in general your formatting is not very idiomatic, with lots of random empty lines and orphaned trailing )).
  • There's a function org-open-file-with-system which I think might do the same as your big org-wiki-xdg-open function
  • helm-core is likely an adequate dependency, rather than all of helm

Hope that helps!

@caiorss
Copy link
Author

caiorss commented Oct 1, 2016

Thanks for the feedback.

M-x checkdoc will help you tidy up docstrings

I've fixed the docstrings.

And in general your formatting is not very idiomatic, with lots of random empty lines and orphaned trailing ))

I have fixed the trailing parenthesis and the empty lines.

There's a function org-open-file-with-system which I think might do the same as your big org-wiki-xdg-open function

I have removed the function org-open-file-with-system, but the function org-wiki-xdg-open didn't do the same as that. It opens any file with system's default application like *.dwg files with Autocad or *.odf spreadsheet with Libreoffice calc.

helm-core is likely an adequate dependency, rather than all of helm

Ok. I did it.

@purcell
Copy link
Member

purcell commented Oct 1, 2016

I've fixed the docstrings.

At least one still has its subsequent lines indented.

I have fixed the trailing parenthesis and the empty lines.

Not as far as I can see. There are still lots of empty random lines before/after (interactive) forms etc, and groups of ) on their own lines.

@tarsius
Copy link
Member

tarsius commented Jan 28, 2017

@caiorss are you still planing on making (pushing?) the requested changes, or should we close this?

@alphapapa
Copy link
Contributor

@caiorss Ping? :)

@tarsius
Copy link
Member

tarsius commented Apr 14, 2017

It has been half a year and I am closing this now. Feel free to reopen when you have the time to finish this.

@tarsius tarsius closed this Apr 14, 2017
@Atlas48
Copy link

Atlas48 commented Aug 1, 2021

As org-wiki appears to be unmaintained, I've decided to make my own fork, atlas48/org-lwiki, and am taking feedback for how to improve the codebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-upstream Awaiting action from an upstream maintainer recipes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants