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

Initial improvements #3

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@steveklabnik
Copy link
Contributor

steveklabnik commented Jan 18, 2012

I broke them out into four different commits in case you only want parts.

  • note about proper ruby version - This project doesn't work without 1.9.3. Added a README note and .rvmrc.
  • Adding test task - Really? I don't want to type 'ruby -Ilib -r./test/helper.rb test/*_test.rb every time I want to run tests. :p
  • adding vim swap files to gitignore - I hate accidentally commiting swap files
  • Changing syntax for errors. Class.new is worse than regular inheritance.
@steveklabnik

This comment has been minimized.

Copy link
Contributor Author

steveklabnik commented Jan 18, 2012

I backed off the commit about the Error situation.

@steveklabnik

This comment has been minimized.

Copy link
Contributor Author

steveklabnik commented Jan 18, 2012

However, I'd accept a patch that adds .rvmrc to the .gitignore and provides a .rvmrc.example file, if you want to send one along.

I can't go along with this. We'll just have to fight to the death over it. Again. :p

@steveklabnik

This comment has been minimized.

Copy link
Contributor Author

steveklabnik commented Jan 18, 2012

I updated the commit to just leave the note in the README, and not do anything with the .rvmrc.

@practicingruby

This comment has been minimized.

Copy link
Member

practicingruby commented Jan 18, 2012

I can't go along with this. We'll just have to fight to the death over it. Again. :p

That's fine. This is what atomic commits are for! They let me ignore things that are clearly wrong 🐨

@steveklabnik

This comment has been minimized.

Copy link
Contributor Author

steveklabnik commented Jan 18, 2012

❤️ ❤️ ❤️

@practicingruby

This comment has been minimized.

Copy link
Member

practicingruby commented Jan 18, 2012

I cherry picked your note about 1.9.3, though I'm not 100% sure its accurate. It may be possible to use this on Ruby < 1.9.3 if you install the io-console gem there. But since I haven't tested at all there, probably better to just mark it 1.9.3+. It's not like anyone is going to make a decision to use this or not based on its ruby version :)

@steveklabnik

This comment has been minimized.

Copy link
Contributor Author

steveklabnik commented Jan 18, 2012

Word. What about the other two?

@eregon

This comment has been minimized.

Copy link
Contributor

eregon commented Jan 18, 2012

Changing syntax for errors. Class.new is worse than regular inheritance.

Why? I really like this syntax, although using the class keyword is fine if in the same file too (but I hate having a file just with class Ex < StEx\nend)

However, I'd accept a patch that adds .rvmrc to the .gitignore and provides a .rvmrc.example file, if you want to send one along.

I can't go along with this. We'll just have to fight to the death over it. Again. :p

I might agree with you here, because rvm users can set whether they want to activate .rvmrc and anyway I use rbenv :)

@practicingruby

This comment has been minimized.

Copy link

practicingruby commented on 74fa55b Jan 18, 2012

Another area where I deviate intentionally from convention. I added a test/suite.rb file though, which will make it easy to run the whole suite.

I just feel like now that I split my gemspec out for Bundler, and I use minitest which allows me to simply require my test files and have them run, Rake is YAGNI until I need it for something. Once I do, this task is a good one to add though.

This comment has been minimized.

Copy link
Owner Author

steveklabnik replied Jan 18, 2012

I'm more concerned about running them easily than using rake specifically, so no specific complaints here.

This comment has been minimized.

Copy link
Owner Author

steveklabnik replied Jan 18, 2012

Oh, one more thing: without a Gemfile, I felt kinda bad not specifying which version of rake we were using. So I actually think I prefer this for that reason.

This comment has been minimized.

Copy link

practicingruby replied Jan 18, 2012

I've been getting away with this "experiment" for about a year now and have not missed Rake. But I've been also mostly working on bullshit toys like this, so YMMV :)

@steveklabnik

This comment has been minimized.

Copy link
Contributor Author

steveklabnik commented Jan 18, 2012

Why?

Please see my commit here for the discussion. I've reversed my position on this.

@practicingruby

This comment has been minimized.

Copy link
Member

practicingruby commented Jan 18, 2012

@steveklabnik Cherry-picked the vim swapfile change, made a similar (but non-conventional) change that addresses the concern of running all the tests at once, without a rake dependency. Closing this request now, because I think that accounts for everything. Feel free to continue discussion though.

@eregon

This comment has been minimized.

Copy link
Contributor

eregon commented Jan 18, 2012

Please see my commit here for the discussion. I've reversed my position on this.

Ah, I see, one of the most weird feature of ruby: constants may "know" their names when assigned a class.

@steveklabnik

This comment has been minimized.

Copy link
Contributor Author

steveklabnik commented Jan 18, 2012

Yep I'd consider it closed.

@practicingruby

This comment has been minimized.

Copy link
Member

practicingruby commented Jan 18, 2012

@eregon:

I might agree with you here, because rvm users can set whether they want to activate .rvmrc and anyway I use rbenv :)

This is precisely the reason why this bugs me :)
If I want to just look around a project, I don't want .rvmrc files to get in my way and ask me if I want to "trust" them. Having a .rvmrc.example file makes it easy for me to copy it in place if I want it, and ignore it otherwise.

@eregon

This comment has been minimized.

Copy link
Contributor

eregon commented Jan 18, 2012

I just feel like now that I split my gemspec out for Bundler, and I use minitest which allows me to simply require my test files and have them run, Rake is YAGNI until I need it for something. Once I do, this task is a good one to add though.

Agreed, rake is just a nice way to group things, but if you have a single task easy enough to run directly (like rspec), it's better to not use Rake, and it's a bit faster and cleaner (but unfortunately TravisCI require both a Gemfile and a Rakefile by default...).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.