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

Implement Convert Post Conditional refactoring #23

Merged
merged 1 commit into from
Aug 30, 2015
Merged

Implement Convert Post Conditional refactoring #23

merged 1 commit into from
Aug 30, 2015

Conversation

mikenichols
Copy link
Contributor

Implements #12

I recently decided I wanted to work on some emacs packages, and this seemed like a good place to start. I'm planning on implementing the remaining refactorings from the vim package, plus any others I think of that I use frequently.

@@ -2,12 +2,13 @@

Ruby refactor is inspired by the Vim plugin vim-refactoring-ruby, currently found at https://github.com/ecomba/vim-ruby-refactoring.

I've implemented 5 refactorings
I've implemented 6 refactorings
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point, it should probably read 'There are refactorings available". :)

@ajvargo
Copy link
Owner

ajvargo commented Aug 16, 2015

Mike! Thanks for the work!

I have a couple asks, though. Could you make this 2 pull requests? One for the misspellings and one for the new feature?

For the new feature, would you mind throwing an example in the 'readme' and adding a feature test? @cheezy was good enough to introduce them as he cleaned things up. If not, do let 'us' know, and I'm sure we can get it sorted.

@mikenichols
Copy link
Contributor Author

Yes, I can totally make this 2 pull requests.

As far as the feature test, I wasn't able to get any of them to run. When I try to run one with cucumber features/extract_constant.feature, I get a bunch of cucumber output that says the scenario is undefined as well as all 10 steps. How can I run them successfully?

Do you want me to clean up the history when I remove the typo fixes? I can remove the commit that's just 'Fix typo' and edit the commit that's got the other typo stuff in it. Or I guess what would be easier: do you want me to just squash all my work into one commit?

@mikenichols
Copy link
Contributor Author

After some research, I installed cask, and set up a Cask file:

(source gnu)
(source melpa)

(package-file "ruby-refactor.el")

(development
 (depends-on "f")
 (depends-on "ecukes")
 (depends-on "espuds")
 (depends-on "ert-runner")
 (depends-on "el-mock"))

I then did a cask install to install the dependencies. When I run cask exec ecukes, I get the following error lines:

Warning: Lisp directory `/usr/local/Cellar/emacs/24.5/share/emacs/24.5/lisp/emacs-parallel'
: No such file or directory                                                               
Warning: Lisp directory `/usr/local/Cellar/emacs/24.5/share/emacs/24.5/lisp/emacs-parallel'
: No such file or directory                                                               
Symbol's value as variable is void: module

I thought this looked like it was trying to read in the features/support/example.rb file as elisp, so I deleted that file, ran cask exec ecukes again and got this output:

Warning: Lisp directory `/usr/local/Cellar/emacs/24.5/share/emacs/24.5/lisp/emacs-parallel'
: No such file or directory                                                               
Warning: Lisp directory `/usr/local/Cellar/emacs/24.5/share/emacs/24.5/lisp/emacs-parallel'
: No such file or directory
(lots of lines describing scenarios)
6 scenarios (6 failed, 0 passed)
52 steps (4 failed, 48 skipped, 0 passed)

At this point, I've spent a couple hours spinning my wheels and I'd like some help to set up my environment correctly so I can write a feature test and get this PR merged.

@ajvargo
Copy link
Owner

ajvargo commented Aug 24, 2015

I'm having the same issues. I assure you it worked once upon a time. :)

If you split it into the 2 PRS - one to fix typos, one to add the feature, I'll get you merged.

Add support for unless as well as if
Update README
@mikenichols
Copy link
Contributor Author

Hey, no worries. It's software :)

I added an example and removed the typo fixes from this PR. I also went ahead and squashed all my commits because I thought that would be cleaner. Let me know if you want me to tweak the wording on anything I touched.

ajvargo pushed a commit that referenced this pull request Aug 30, 2015
Implement Convert Post Conditional refactoring
@ajvargo ajvargo merged commit 32e9767 into ajvargo:master Aug 30, 2015
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.

2 participants