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

Creating rspec-its gem for rspec-core issue #1083 #1

Merged
merged 22 commits into from
Oct 2, 2013
Merged

Creating rspec-its gem for rspec-core issue #1083 #1

merged 22 commits into from
Oct 2, 2013

Conversation

palfvin
Copy link
Contributor

@palfvin palfvin commented Sep 28, 2013

Submitted for code review.

@@ -0,0 +1,2 @@


Copy link
Member

Choose a reason for hiding this comment

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

What's this file for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was keeping track of some things I wanted to bring up with you, but I ended up answering them all myself prior to the last commit. I'll get rid of it and use github mechanisms going forward.

@@ -0,0 +1 @@
--color
Copy link
Member

Choose a reason for hiding this comment

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

We should also run the rspec repo's with --order rand and --warnings

@ghost ghost assigned palfvin Sep 28, 2013
@palfvin
Copy link
Contributor Author

palfvin commented Oct 1, 2013

Don't mean to press, guys, but wanted to make sure you'd seen #1 (comment) If you don't know what to do about it off hand, let me know and I'll dig into it.

@palfvin
Copy link
Contributor Author

palfvin commented Oct 1, 2013

I've been getitng the following warning when I run the rspec examples:

/Users/palfvin/.rvm/rubies/ruby-2.0.0-p247/lib/ruby/2.0.0/openssl/ssl.rb:101: warning: assigned but unused variable - id

and I'm also getting three warnings about rspec-core redefining constants that are previously defined in rspec-expectations. You can see them at https://travis-ci.org/palfvin/rspec-its/jobs/11993819

Are those anything I need to worry about?

@JonRowe
Copy link
Member

JonRowe commented Oct 1, 2013

The openssl warnings are, afaik, something we can't do anything about. I've looked at them before. The redefining constants warnings might be fixable, but they're not anything to worry about.

matrix:
allow_failures:
- rvm: rbx-19mode
- rvm: rbx-18mode
Copy link
Member

Choose a reason for hiding this comment

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

Still an incorrect line ending here ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for persisting. ;-) Turns out I didn't realize glob didn't handle . files. Then I realized I didn't really understand how to match an empty string. Anyway, next commit will be "clean" in this respect.

@JonRowe
Copy link
Member

JonRowe commented Oct 1, 2013

Looks good to me, I left a few more notes but they're mostly about formatting.

@myronmarston
Copy link
Member

Looks good...merge away whenever you want to :).

palfvin added a commit that referenced this pull request Oct 2, 2013
Creating rspec-its gem for rspec-core issue #1083
@palfvin palfvin merged commit 6954dac into rspec:master Oct 2, 2013
@palfvin
Copy link
Contributor Author

palfvin commented Oct 2, 2013

Sorry for displaying yet more naivete, but what are the next steps for publishing this to rubygems? I see that there's a version of rspec_collection-matchers out there, but it's got a lot of oddities, including:

  • You (Myron) as creator, not the gem author
  • A documentation link at the top that's broken
  • No set of source, documentation, etc. links on the bottom like rspec-core

And do you want me to go ahead and submit the pull request for the 3.0 extraction or do you want to get the rubygems things squared aware first?

@JonRowe
Copy link
Member

JonRowe commented Oct 2, 2013

@palfvin that will be myron just place holdering the gem, you can start on the PR for extraction and Myron will sort the gem deployment out with you at some point.

Note I like to check that this sort of thing works in the following way, create some specs in an empty gemset that follow this pattern. It's ok to do this manually.

| 2.14           | Works            |
| 2.99           | Work but warn    |
| 2.99 + gem     | Work, no warning |
| 3.x            | Fail             |
| 3.x + gem      | Work             |

palfvin pushed a commit that referenced this pull request Mar 1, 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.

None yet

3 participants