Skip to content

Conversation

jgebal
Copy link
Contributor

@jgebal jgebal commented Nov 11, 2015

Call to plsql-spec init now creates additional file:
.rspec in the project root directory.

The file contains a require, so the spec_helper is always required.
This way when writing tests, the spec helper no longer needs to be explicitly required.

@jgebal
Copy link
Contributor Author

jgebal commented Nov 17, 2015

@javornikolov any concerns/comments on this change?

@javornikolov
Copy link
Collaborator

I think --requrie option was introduced with rspec 3.x (and perhaps the transitional 2.99 version too). So I'm wondering whether we should do something to keep things compatible.

empty_directory 'spec/factories'
directory 'spec', 'spec'
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is that doing? (Is it related to the require spec_helper change?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is because the templates directory contained the content of spec directory to initialise project with.
After the change, project is simply initialised with the spec folder contained within the templates.
Additionally a file .rspec is added to the project root.
All the files that existed in lib/plsql/spec/templates were moved to lib/plsql/spec/templates/spec

@jgebal
Copy link
Contributor Author

jgebal commented Nov 17, 2015

Good spot. I did not check before. The --require option was included since 2.14
What is the policy for backward compatibility in this project?
Do you think it would be worth/advisable to use the most recent features of underlying libraries, even if it means bumping the project version by a number to indicate it's no longer backward-compatible?

…le: .rspec

The file contains a require, so the spec_helper is always required.
This way when writing tests, the spec helper no longer needs to be explicitly required.
@jgebal jgebal force-pushed the feature/get_rid_of_spec_helper_references branch from bf4126b to f8d10e2 Compare November 18, 2015 00:17
empty_directory 'spec/factories'
copy_file '.rspec', '.rspec'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed a bug. The file though defined was not copied by cli.rb

@jgebal
Copy link
Contributor Author

jgebal commented Nov 18, 2015

@javornikolov I've added fixes and a test for new file.

@javornikolov
Copy link
Collaborator

Good spot. I did not check before. The --require option was included since 2.14
What is the policy for backward compatibility in this project?
Do you think it would be worth/advisable to use the most recent features of underlying libraries, even if it means bumping the project version by a number to indicate it's no longer backward-compatible?

I think there is no strict policy for backward compatibility. It may be tricky - depends on users' environments.

But to be honest - in this case:

  • rspec 2.x seems to survive unknown extra options in .rspec
  • the examples are already using expect syntax which is not available in rspec 2 (so it anyway needs some tweaking to adapt to older versions). But these are just examples

Also I think rspec < 2.4 is not working with latest ruby.

So I guess we're not breaking anything significant by this change.

@jgebal
Copy link
Contributor Author

jgebal commented Nov 18, 2015

All test were passing after my latest fix.
Do you think it's worth merging?
I have a backlog of small tweaks and extensions, so few pull requests might be coming soon.

@javornikolov
Copy link
Collaborator

:shipit: Merging... @jgebal , thank you for adding this enhancement!

@jgebal
Copy link
Contributor Author

jgebal commented Nov 19, 2015

Thanks. It's a pleasure.

javornikolov added a commit that referenced this pull request Nov 19, 2015
…erences

Refactored, so the call to "require 'spec_helper'" is no longer needed in each spec file.
@javornikolov javornikolov merged commit 96e6312 into rsim:master Nov 19, 2015
@javornikolov javornikolov mentioned this pull request May 12, 2016
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