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

Update lua-mode recipe #6219

Merged
merged 2 commits into from
Jun 29, 2019
Merged

Update lua-mode recipe #6219

merged 2 commits into from
Jun 29, 2019

Conversation

immerrr
Copy link
Contributor

@immerrr immerrr commented Jun 11, 2019

There's an upcoming change that would require to accompany the source code with a Lua initializer script for the inferior shell. This is to ensure the script is redistributed accordingly.

Please note, that scripts directory is not there yet, and I'm not sure if the recipe works without it. If that's the case we can wait with merging until the scripts directory is created.

Brief summary of what the package does

lua-mode is a major mode for editing Lua files

Direct link to the package repository

https://github.com/immerrr/lua-mode

Your association with the package

I'm the maintainer.

Relevant communications with the upstream package maintainer

No package.el compatibility changes were introduced.

Checklist

Please confirm with x:

  • The package is released under a GPL-Compatible Free Software License.
  • I've read CONTRIBUTING.org
  • I've used the latest version of package-lint to check for packaging issues, and addressed its feedback
  • My elisp byte-compiles cleanly
  • M-x checkdoc is happy with my docstrings
  • I've built and installed the package using the instructions in CONTRIBUTING.org
  • I have confirmed some of these without doing them

Looks like, lua-mode is quite far behind on the package quality side of things, but if it's OK I would prefer to fix it in a separate PR.

There's an [upcoming change](immerrr/lua-mode#148) that would require to accompany the source code with a Lua initializer script for the inferior shell. This is to ensure the script is redistributed accordingly. 

Please note, that `scripts` directory is not there yet, and I'm not sure if the recipe works without it. If that's the case we can wait with merging until the scripts directory is created.
@tarsius
Copy link
Member

tarsius commented Jun 17, 2019

Looks like, lua-mode is quite far behind on the package quality side of things, but if it's OK I would prefer to fix it in a separate PR.

That's reasonable but I would suggest you do the "I've built and installed the package using the instructions in CONTRIBUTING.org" step in order to answer your question "[the] scripts directory is not there yet, and I'm not sure if the recipe works without it."

@tarsius
Copy link
Member

tarsius commented Jun 17, 2019

Err actually... we know that the build part succeeded and the recipe does not affect installation, so all good.

I am not sure what the best value for :files would be in this case though, so I won't merge.

@riscy
Copy link
Member

riscy commented Jun 22, 2019

@immerrr can you ping when the scripts directory is set up? Then we can verify the files you're specifying are being laid out exactly how you want.

You can refer to the recipe for auto-complete:

(auto-complete :repo "auto-complete/auto-complete" :fetcher github :files
               ("*.el" "dict"))

which creates a file tree that looks like this:

├── auto-complete-20170125.245   
   ├── auto-complete-autoloads.el
   ├── auto-complete-config.el   
   ├── auto-complete-config.elc  
   ├── auto-complete-pkg.el      
   ├── auto-complete.el          
   ├── auto-complete.elc         
   └── dict                      

...where the dict directory is full of files.

@riscy riscy added the awaiting-upstream Awaiting action from an upstream maintainer label Jun 22, 2019
@purcell
Copy link
Member

purcell commented Jun 29, 2019

A better (and equivalent) :files pattern would be (:defaults "scripts"). I'll make that change and merge this.

@purcell purcell merged commit 5f3938b into melpa:master Jun 29, 2019
@immerrr
Copy link
Contributor Author

immerrr commented Jun 29, 2019

Thank you @purcell

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants