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 helm-elscreen #4828

Merged
merged 1 commit into from Jul 9, 2017
Merged

Add helm-elscreen #4828

merged 1 commit into from Jul 9, 2017

Conversation

masutaka
Copy link
Contributor

@masutaka masutaka commented Jul 2, 2017

helm-elscreen.el was moved by emacs-helm/helm@6193cc4 to https://github.com/emacs-helm/helm-elscreen. So I want to add it to melpa.

Brief summary of what the package does

Elscreen with Helm interface

Direct link to the package repository

https://github.com/emacs-helm/helm-elscreen

Your association with the package

I'm an enthusiastic user.

Relevant communications with the upstream package maintainer

None needed

Checklist

  • I've read CONTRIBUTING.md
  • I've used 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.md

@purcell
Copy link
Member

purcell commented Jul 9, 2017

Thanks. Sorry for the delay in reviewing this. Quick feedback:

  • The new package should declare package dependencies on helm and elscreen, and then use require 'elscreen to avoid needing declare-function.
  • There should also be a dependency declared on cl-lib.
  • The package description "Elscreen support" should be changed to mention helm, e.g. something like Switch screens in elscreen using helm.

Hope that helps!

@masutaka
Copy link
Contributor Author

masutaka commented Jul 9, 2017

Thanks for your review. I've just fixed in emacs-helm/helm-elscreen#2.

@purcell
Copy link
Member

purcell commented Jul 9, 2017

Thanks. Just this little point to fix now. :-)

@masutaka
Copy link
Contributor Author

masutaka commented Jul 9, 2017

The little point was fixed in emacs-helm/helm-elscreen@b821286.

I believe all problems were fixed. Can you merge this?

@purcell purcell merged commit dfe42a7 into melpa:master Jul 9, 2017
@purcell
Copy link
Member

purcell commented Jul 9, 2017

Great, done. Thanks!

@masutaka masutaka deleted the helm-elscreen branch July 9, 2017 11:42
microamp pushed a commit to microamp/melpa that referenced this pull request Jul 24, 2017
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

Successfully merging this pull request may close these issues.

None yet

2 participants