solidusio / solidus_dev_support

A collection of tools for developing Solidus extensions.
MIT License
21 stars 27 forks source link

Fix sandbox default solidus branch #192

Closed RyanofWoods closed 1 year ago

RyanofWoods commented 1 year ago

Fixes the sandbox.tt file script not defaulting to the master Solidus when no SOLIDUS_BRANCH variable is given.

Summary

When an uninitialized variable is given to -n, it is treated as not NULL. The variable must be quoted for correct results. [ -n "$SOLIDUS_BRANCH" ]

It is also recommended for variable comparisons in general as it can produce incorrect results for ! -z.

References:

This meant that before, when running this file with no SOLIDUS_BRANCH variable, $BRANCH would end up NULL and the branch in the Gemfile would be an empty string. This caused an error, whereas instead, it should have defaulted to master.

Error:

fatal: Needed a single revision
Git error: command `git rev-parse --verify ''` in directory /Users/ryanwoods/.rbenv/versions/2.7.6/lib/ruby/gems/2.7.0/cache/bundler/git/solidus-169f1ecb1aee2122950e6d586daf2410f62df66e has
failed.
Revision  does not exist in the repository https://github.com/solidusio/solidus.git. Maybe you misspelled it?
If this error persists you could try removing the cache directory
'/Users/ryanwoods/.rbenv/versions/2.7.6/lib/ruby/gems/2.7.0/cache/bundler/git/solidus-169f1ecb1aee2122950e6d586daf2410f62df66e'

The git source https://github.com/solidusio/solidus.git is not yet checked out. Please run `bundle install` before trying to start your application

Checklist

To test this, I commented out half of the sandbox.tt file after (extension_name="<%= file_name %>") and ran:

bash lib/solidus_dev_support/templates/extension/bin/sandbox.tt

It logged that the master branch was being used and solidus_frontend and no debug mode was used.

I then ran:

DEBUG=true SOLIDUS_BRANCH="v3.2" SOLIDUS_FRONTEND="solidus_starter_frontend" bash lib/solidus_dev_support/templates/extension/bin/sandbox.tt

Which logged that the v.3.2 Solidus branch was being used and solidus_starter_frontend for the frontend πŸ‘ It also correctly used the debug mode.

Screenshot 2022-11-09 at 10 19 50
mergify[bot] commented 1 year ago

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:

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

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

RyanofWoods commented 1 year ago

It would probably be good to update some extension's sandbox script after this change. Note, I may soon make another PR to change the sandbox.tt further, so we may want to wait πŸ‘


As another note, from my understanding, each extension copies the sandbox.tt file, but maybe it would reduce overhead, if extensions could just delegate to using the solidus_dev_support sandbox script if it doesn't need to modify it, so it doesn't need updating if changes are made to the main one. πŸ€” On the other hand, changes to the main one could break the extension's sandbox setup.

RyanofWoods commented 1 year ago

I am unable to change the label @waiting-for-dev

waiting-for-dev commented 1 year ago

As another note, from my understanding, each extension copies the sandbox.tt file, but maybe it would reduce overhead, if extensions could just delegate to using the solidus_dev_support sandbox script if it doesn't need to modify it, so it doesn't need updating if changes are made to the main one. πŸ€” On the other hand, changes to the main one could break the extension's sandbox setup.

Yeah, at this point, the purpose of the sandbox application is to be used as a starting point, so I wouldn't change anything for now.