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

Mavenlink upgrades #1

Merged
merged 21 commits into from
May 11, 2015
Merged

Mavenlink upgrades #1

merged 21 commits into from
May 11, 2015

Conversation

randycoulman
Copy link

This is a fairly large PR that contains a number of upgrades that @cantino made to this somewhat outdated gem:

  • Upgrade to rspec 2.10 to match what we use on bigmaven
  • Upgrade to Rails 3+ only (dropping support for Rails 2)
  • Upgrade to Ruby 1.9.3+ only (dropping support for 1.8.7)
  • Split the specs into separate files
  • Allow the parent and child relations to be customized with new options
  • Protect against SQL injection
  • Rename ancestry_string to materialized_path to avoid confusion with the ancestry gem
  • Write more specs around trees of unsaved objects, and fix a few bugs that we found with them

@bobby1190 @shirish-pampoorickal: Can you look this over and see what you think?

@randycoulman
Copy link
Author

Also need to:

  • Update the README

@cantino
Copy link

cantino commented May 6, 2015

I added Travis, maybe we need to re-push this?

@randycoulman
Copy link
Author

Closing and re-opening to kick the Travis build

@randycoulman randycoulman reopened this May 6, 2015
@cantino
Copy link

cantino commented May 7, 2015

I don't think we should commit the .pairs file publicly. We can add a global one to ansible, but for now, please remove and force push.

@randycoulman randycoulman force-pushed the mavenlink_upgrades branch from 5a8c95e to 18e4d9d Compare May 7, 2015 18:05

class Thing < ActiveRecord::Base

acts_arboreal parent_relation_options: { touch: true }, children_relation_options: { autosave: true }
Copy link

Choose a reason for hiding this comment

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

nice

@randycoulman
Copy link
Author

@cantino Can we get a final LGTM on this?

@cantino
Copy link

cantino commented May 11, 2015

LGTM!

randycoulman added a commit that referenced this pull request May 11, 2015
@randycoulman randycoulman merged commit 4e0abee into master May 11, 2015
@randycoulman randycoulman deleted the mavenlink_upgrades branch May 11, 2015 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants