Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add johanvts/emacs-fireplace #3403

Merged
merged 1 commit into from Dec 20, 2015

Conversation

Projects
None yet
4 participants
Contributor

hut8 commented Dec 18, 2015

https://github.com/johanvts/emacs-fireplace

What it does:

displays a fireplace

My relation to project

I asked if I could make a package for it here: johanvts/emacs-fireplace#1

Thanks.

@hut8 hut8 referenced this pull request in johanvts/emacs-fireplace Dec 18, 2015

Closed

[Request] Adding in melpa repo please #5

Contributor

AdrieanKhisbe commented Dec 18, 2015

👍

Owner

milkypostman commented Dec 18, 2015

haha, this is hilarious. but it's missing an autoload statement which means it will be worthless to add to MELPA at this point.

can you send a PR/file an issue with the original package to add the autoloads, then we can merge this.

I think all you need is to add,

;;;### autoload

before the fireplace function.

Contributor

hut8 commented Dec 18, 2015

Aww man, rookie mistake. I'm on it.

hut8 added a commit to hut8/emacs-fireplace that referenced this pull request Dec 18, 2015

Contributor

hut8 commented Dec 18, 2015

Heh. Beat me to it @AdrieanKhisbe

Owner

purcell commented Dec 18, 2015

This is lots of fun, so please bear with me while I get a little picky:

  • The metadata looks like it's been hacked together just enough to work, but it's messy, with the Commentary: above the other headers etc. Please try using M-x auto-insert in a fresh fireplace.el and then use the more complete boilerplate it will generate for you.
  • fp- is not a good prefix for private symbol names. Use fireplace--, as per the elisp conventions.
  • C-s is a keybinding reserved for users
Owner

purcell commented Dec 18, 2015

Also: smoke, flame, make-grid, gotoxy etc. all need to be prefixed with fireplace- or fireplace--.

@AdrieanKhisbe AdrieanKhisbe referenced this pull request in johanvts/emacs-fireplace Dec 19, 2015

Merged

Autoloads cookie and minor refactors #8

Contributor

AdrieanKhisbe commented Dec 19, 2015

@purcell: your concern have now been addressed and PR has been merge.

I thinks it's time to put melpa on fire!! :D 🔥

purcell added a commit that referenced this pull request Dec 20, 2015

Merge pull request #3403 from hut8/master
Add johanvts/emacs-fireplace

@purcell purcell merged commit 4c1ac52 into melpa:master Dec 20, 2015

1 check passed

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

purcell commented Dec 20, 2015

Thanks!

Contributor

AdrieanKhisbe commented Dec 20, 2015

🎆

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