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

Add system specs configuration #3552

Merged
merged 2 commits into from
Mar 20, 2020
Merged

Add system specs configuration #3552

merged 2 commits into from
Mar 20, 2020

Conversation

blocknotes
Copy link
Contributor

@blocknotes blocknotes commented Mar 13, 2020

Description

Replicate the feature specs configuration and helpers for system specs.

Adding system specs support can be a good enhancement for extensions mainly.

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have updated Guides and README accordingly to this change (if needed)
  • I have added tests to cover this change (if needed)
  • I have attached screenshots to this PR for visual changes (if needed)

Replicate the feature specs configuration and helpers for system specs.
@blocknotes blocknotes marked this pull request as ready for review March 13, 2020 10:40
Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

Thanks @blocknotes, I think this works but I'd avoid defining methods in the top level context of the spec_helper. What about moving them into spree/testing_support/ files split by functionality?

Something like:

spree/testing_support/print_missing_translation
spree/testing_support/blacklist_urls
spree/testing_support/prepare_assets_for_browser

This way we can add them safely when needed without duplication, even in stores and extensions. Thoughts?

@blocknotes
Copy link
Contributor Author

This way we can add them safely when needed without duplication, even in stores and extensions. Thoughts?

@kennyadsl: good idea, I have just pushed a commit to isolate the new helpers.

Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

👍 Thanks Mattia!

Copy link
Member

@spaghetticode spaghetticode left a comment

Choose a reason for hiding this comment

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

@blocknotes thank you 👍

@kennyadsl kennyadsl merged commit 6a21551 into solidusio:master Mar 20, 2020
blocknotes added a commit to solidusio/solidus_starter_frontend that referenced this pull request Apr 10, 2020
From version 2.11 the systems specs option will be included.
PR: `solidusio/solidus#3552`
blocknotes added a commit to solidusio/solidus_starter_frontend that referenced this pull request Apr 10, 2020
From version 2.11 the systems specs option will be included.
PR: `solidusio/solidus#3552`
blocknotes added a commit to solidusio/solidus_starter_frontend that referenced this pull request Apr 10, 2020
From version 2.11 the systems specs option will be included.
PR: `solidusio/solidus#3552`
blocknotes added a commit to solidusio/solidus_starter_frontend that referenced this pull request Apr 10, 2020
From version 2.11 the systems specs option will be included.
PR: `solidusio/solidus#3552`
blocknotes added a commit to solidusio/solidus_starter_frontend that referenced this pull request May 25, 2020
From version 2.11 the systems specs option will be included.
PR: `solidusio/solidus#3552`
blocknotes added a commit to solidusio/solidus_starter_frontend that referenced this pull request May 25, 2020
From version 2.11 the systems specs option will be included.
PR: `solidusio/solidus#3552`
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.

3 participants