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

Composer install script #8

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

Composer install script #8

wants to merge 3 commits into from

Conversation

woodseowl
Copy link
Collaborator

@woodseowl woodseowl commented Jul 6, 2020

Here is an idea I am working on for taking care of all of the instructions for file renames in the readme.

It has a dependency on the unix sed command, so I'm not sure how portable it is, but it works on my Mac.

@woodseowl woodseowl requested review from alisonjo315 and ama39 July 6, 2020 17:35
@alisonjo315
Copy link
Collaborator

This is so cool! I can't wait to try it! Next week 🤞 🤞

@alisonjo315
Copy link
Collaborator

alisonjo315 commented Apr 4, 2021

Finally taking a look -- just a few weeks later 😆

I tried it out using the instructions you provided in the README, with one modification that only matters for testing: composer require cubear/cwd_project:dev-composer-script
(sharing ^^this tip in case anyone else comes in to test)

It's really cool, thank you for writing and proposing!

So I'm putting feedback in one running-but-sectioned list...


Suggestions for the README:

  1. Remove the > characters -- it's nice to be able to "triple-click highlight" and copy/paste commands.
  2. "SITE_NAME" might should be "THEME_NAME", and "THEME" might could be "THEME_SHORT_NAME" (or THEME_MACHINE_NAME).
  3. Also, it might be implied for some people, but it wasn't obvious to me that the text in [] needed quotes around it, maybe add quotes?
  4. Maybe use the "ursine" example that's used in the rest of the README -- i.e.
export THEME_NAME="College of Ursine Studies theme"
export THEME_SHORT_NAME="ursine"

(see #6 below for my short name question/suggestion)


Question:

  1. How long do bash variables last? 'Til your next session, or longer, or not that long? -- if they last a while, I wonder if the variable names should be less generic? (if they don't last a while, no need / never mind)

Running the script:

  1. sed existed on my Ubuntu machine, yay, but the script didn't work as-is for me, I had to remove the "in-place" flag. When I ran it as-is, it stopped with the very first command, with this error:
$ composer install-project
> sed -i '' "s/cwd_project/$THEME/g" cwd_project.info.yml
sed: can't read s/cwd_project/cwd_batty/g: No such file or directory
Script sed -i '' "s/cwd_project/$THEME/g" cwd_project.info.yml handling the install-project event returned with error code 2

(When I removed -i '' from each sed command, the script worked. I haven't used sed at all, so idk why/what's up, but, that's what happened when I tried it out!)

  1. When the script runs, it outputs everything to terminal. Does it need to do that or is it optional? It was just, a lot of lines output on my screen, making scroll-back less than ideal.

Other thoughts:

  1. It's nice that the js/scss files don't have to have the full machine name of the theme. Maybe the prompt could use a shorter machine name, so to speak, and insert it after "cwd_" in most cases, but leave it as-is for those two files? -- i.e. something like...
    export THEME_MACHINENAME="[YOUR CUSTOM THEME SHORT NAME]"
    Then, the script could replace instances of "cwd_project" with "cwd_YOUR_SHORT_NAME", and rename "project.js" "YOUR_SHORT_NAME.js".

  2. What would you think about ending the script with cd .. ? -- I was disoriented when it was done, 'til I realized I was in a directory that didn't really exist anymore :D To be fair, if I'd read every line of the script beforehand, I would've realized it was renaming the directory along with everything else, and I might not have been so "lost".

  3. After running the script, there were just two straggler cwd_project instances, both in .info.yml:

$ grep -r "cwd_project" cwd_batty/
cwd_batty/cwd_batty.info.yml:  - 'cwd_project/global-styling' # @CUSTOMIZE
cwd_batty/cwd_batty.info.yml:  - 'cwd_project/typekit' # @CUSTOMIZE

Copy link
Collaborator

@alisonjo315 alisonjo315 left a comment

Choose a reason for hiding this comment

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

(reviewed; comments in previous comment, oops)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants