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

Remove some duplication #78

Closed
wants to merge 6 commits into from
Closed

Remove some duplication #78

wants to merge 6 commits into from

Conversation

nusco
Copy link
Contributor

@nusco nusco commented Mar 23, 2013

Just refactorings, and added a .travis.yml file. Warning: this pull breaks compatibility with Ruby 1.8. You may want to refuse it or pull to a branch if you care about that. Rails will drop 1.8 in the next minor release, so the time to pull the plug on 1.8 might have come.

@iGEL
Copy link
Contributor

iGEL commented Aug 30, 2014

I'm sorry, I think it's better to fix duplication without meta programming (see #111). This causes the methods to not be documented anymore and standard definitions are easier to grep for and parse for IDEs.

@nusco
Copy link
Contributor Author

nusco commented Aug 30, 2014

Yup, it's a trade-off. I don't use an IDE when coding Ruby, so I have a bias towards shorter code.

@junaruga
Copy link
Contributor

@nusco thank you for your pull-request. sorry for late response.
some of this PR were merged to master branch.

  • Adding .travis.yml.
  • A part of Gemfile was updated.
  • we are going to delete Gemfile.lock soon in another PR.

How to fix the duplication. I am considering. There are 3 PRs for that, as far as I know.

@nusco
Copy link
Contributor Author

nusco commented Apr 27, 2017

@junaruga, no problem, and thanks for checking this! I didn't even remember this PR, and I guess most of it is obsolete by now. Just take what you need and leave the rest. 😃

dennissivia added a commit that referenced this pull request May 15, 2017
This PR implements PRs #65, #74 and #78.
Some of the PRs contain similar content and other are no longer
based on the current readme. THis is an attempt to clean up the
readme state. In addition some suggestions for further updates
are part of the PR to raise the discussion about:
List of maintainers and the format (email or github username) and
the copyright text we want to have (or not have).
@perlun
Copy link
Contributor

perlun commented Jun 6, 2017

@junaruga Will you finish this one? If you do, please submit another PR and close this one. Leaving open for now to give room for some discussion.

@perlun
Copy link
Contributor

perlun commented Jul 14, 2017

Closing for lack of feedback. We can reopen if the OP reappears.

@perlun perlun closed this Jul 14, 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.

4 participants