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

Update support to latest requirements for extensions #177

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

cpfergus1
Copy link
Contributor

Summary

Updated Readme with more explicit shell command for running solidus extension .
Removed rails > 0.a, bundler has been patched to prevent the issue with resolving dependencies.
Bumped Ruby from 2.5 to 2.6, Ruby 2.5 EOL was April 1st 2021, and 2.6 EOL is slated for March 31st 2022

Checklist

  • I have structured the commits for clarity and conciseness.
  • I have added relevant automated tests for this change.

@mergify mergify bot added the needs changelog label Needs a label to determine the type of change. label Jun 16, 2021
@mergify
Copy link
Contributor

mergify bot commented Jun 16, 2021

It looks like this PR is missing a label to determine the type of change it introduces. The maintainer should add one of the following labels:

  • bug for bugfixes.
  • enhancement for new features and improvements.
  • documentation for documentation changes.
  • security for security patches.
  • removed for feature removals.
  • infrastructure for internal changes that should not go in the changelog.

Additionally, the maintainer may also want to add one of the following:

  • breaking for breaking changes.
  • deprecated for feature deprecations.

Once the correct labels have been set, simply remove the needs changelog label label from this PR so I can merge it.

@cpfergus1 cpfergus1 force-pushed the update-support-to-latest-requirements-for-extensions branch from 4f667fb to 53c5363 Compare June 16, 2021 18:40
@elia elia force-pushed the update-support-to-latest-requirements-for-extensions branch from 53c5363 to f8cbc61 Compare September 6, 2022 13:04
@elia
Copy link
Member

elia commented Sep 6, 2022

@cpfergus1 I rebased this on top of #188, since they were touching similar files, once that's merged we can rebase on master and merge it if everything checks.

@elia elia force-pushed the update-support-to-latest-requirements-for-extensions branch from f8cbc61 to 8085b28 Compare September 6, 2022 15:18
Ruby 2.5 has reached EOL and will no longer recieve any security
patches. Bumped version requirements to 2.6 as it is best practiced to use
a maintained version of Ruby. Ruby 2.6 EOL slated for March 31, 2022
The bug that this code fixed has been patched in the the latest versions
of bundler and is no longer required.
The command `solidus extension .` may not work directly on some machines
without `bundle exec`. Updated to be more explicit and prevent possible
confusion.
@elia elia force-pushed the update-support-to-latest-requirements-for-extensions branch from 8085b28 to c5bcda0 Compare September 9, 2022 07:48
@elia elia added enhancement Improves an existing feature. infrastructure Internal change — changelog entry not needed. and removed needs changelog label Needs a label to determine the type of change. labels Sep 9, 2022
Copy link
Contributor

@forkata forkata left a comment

Choose a reason for hiding this comment

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

Thanks for these changes @cpfergus1 and @elia!

Comment on lines -13 to -15
# A workaround for https://github.com/bundler/bundler/issues/6677
gem 'rails', '>0.a'

Copy link
Contributor

Choose a reason for hiding this comment

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

One thing we've noticed on solidus_taxjar when we removed Rails from the Gemfile is that it exposed an issue upstream with active_storage. It is explicitly required here however it isn't listed as one of the Rials gems that are dependencies of core here. I still think this change is good, however if you see failures booting the dummy app, this could be the culprit. I believe this should be fixed upstream in core however.

@@ -42,7 +42,7 @@ If you have an existing extension and want to update it to use the latest standa
you can run the following in the extension's directory:

```console
$ solidus extension .
$ bundle exec solidus extension .
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for clarifying this!

I think we may want to also clarify in the section above on generating a new extension, that this gem needs to be installed globally (i.e. gem install solidus_dev_support in order for the the solidus executable to be added to the path. The installation instructions only focus on adding this as a dependency of an existing extension, which I think is causing some of the confusion.

@stale
Copy link

stale bot commented Nov 14, 2022

This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated.

@stale stale bot added the Stale label Nov 14, 2022
@stale
Copy link

stale bot commented Nov 21, 2022

Closing this pull request after a prolonged period of inactivity. If this issue is still present in the latest release, please ask for this pull request to be reopened. Thank you!

@stale stale bot closed this Nov 21, 2022
@kennyadsl kennyadsl reopened this Nov 22, 2022
@stale stale bot removed the Stale label Nov 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improves an existing feature. infrastructure Internal change — changelog entry not needed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants