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

Refactor Schemas into LinkedDataFragments Namespace #39

Merged
merged 4 commits into from
Feb 3, 2017

Conversation

no-reply
Copy link
Member

@no-reply no-reply commented Feb 2, 2017

Moves Schemas into lib as a first step in isolating other logic.

Tom Johnson added 4 commits February 1, 2017 09:56
Adds a generic Guardfile.

`spec_helper.rb` includes a stub on `Setting` in a before(:each), which
prevents various spec files from running without the rest of the
suite. This pattern also has the issue that it stubs
`Setting#cache_backend` without warning to developers, giving unexpected
results. We should find a way to remove it. See #38.

In the meanwhile, we introduce a guard to prevent errors, and allow
modules that don't depend on `Setting` to be tested independently.
Replaces `app` schemas with those in `lib`. This breaks backward
compatibility for any clients referring directly to the schema
constants. It's otherwise a simple refactor.
This is the first step in a refactoring to move schemas to lib. Adds
tests for the schemas.
@scande3
Copy link
Contributor

scande3 commented Feb 2, 2017

Is this in reference to #38 so that you can run individual tests?

@no-reply
Copy link
Member Author

no-reply commented Feb 2, 2017

@scande3: It's the first step in a stepwise refactor to move core logic to lib. There are follow-up PRs on the way to make the core logic sharable, make the application mountable, and gemify the project.

I'm submitting them in the small refactors like this to keep review sane and ensure we avoid breaking changes.

#38 was encountered along the way. The aggressive stubbing made it hard to extract Setting from the rails app.

@no-reply
Copy link
Member Author

no-reply commented Feb 2, 2017

Copy link
Contributor

@scande3 scande3 left a comment

Choose a reason for hiding this comment

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

I'm not entirely sure I understand the full need for this refactor but as it doesn't break anything that I can see, going to approve it. Nothing incorrect about any of the code that I noticed. Thanks!

@scande3 scande3 merged commit 56245b5 into master Feb 3, 2017
@no-reply no-reply deleted the feature/refactor-schemas branch February 3, 2017 22:24
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