Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

It's worth starting with a shebang; I'd also advise setting shell flags -e (exit on error) and -u (error when using unset variables):

#!/bin/bash
set -eu

Please use more whitespace and comments to make it easier to follow.

Perl is overkill for the simple substitution here - if we stick to sed, then that's one fewer dependency:

sed -e '/^extract_css:/:.*/ true/' \
 -i config/webpacker.yml

I omitted creating the backup because I don't think there's any value in keeping the original. If your later processes depend on having it around, then use -i.bak instead of -i, as in Perl (in fact, that's where Perl got that option).

The two transform-and-move pairs can also use sed -i, making the script more consistent, and thus easier to follow. We can use the (standard sed) r command to read a file, instead of the non-standard e command:

sed -e '/module.exports = environment/rexpose-loader.txt' \
 -i config/webpack/environment.js
sed -e '/javascript_pack_tag .application/rstylesheet_pack_tag.txt' \
 -i app/views/layouts/application.html.erb

$s1 is only used once, and doesn't add any value (such as a meaningful name), so just inline it.

If we can eliminate the one use of C-escape quoting, then we can use plain POSIX shell instead of Bash.


#Modified code #!/bin/sh

Modified code

#!/bin/sh
set -eu
yarn add bootstrap jquery popper.js expose-loader
yarn install --check-files
sed -e '/^extract_css:/:.*/ true/' \
 -i config/webpacker.yml
printf '\n\n%s' 'import "bootstrap/dist/js/bootstrap";' \
 >> app/javascript/packs/application.js
echo '@import "~bootstrap/scss/bootstrap";' \
 > app/javascript/packs/styles.scss
sed -e '/module.exports = environment/rexpose-loader.txt' \
 -i config/webpack/environment.js
sed -e '/javascript_pack_tag .application/rstylesheet_pack_tag.txt' \
 -i app/views/layouts/application.html.erb

It's worth starting with a shebang; I'd also advise setting shell flags -e (exit on error) and -u (error when using unset variables):

#!/bin/bash
set -eu

Please use more whitespace and comments to make it easier to follow.

Perl is overkill for the simple substitution here - if we stick to sed, then that's one fewer dependency:

sed -e '/^extract_css:/:.*/ true/' \
 -i config/webpacker.yml

I omitted creating the backup because I don't think there's any value in keeping the original. If your later processes depend on having it around, then use -i.bak instead of -i, as in Perl (in fact, that's where Perl got that option).

The two transform-and-move pairs can also use sed -i, making the script more consistent, and thus easier to follow. We can use the (standard sed) r command to read a file, instead of the non-standard e command:

sed -e '/module.exports = environment/rexpose-loader.txt' \
 -i config/webpack/environment.js
sed -e '/javascript_pack_tag .application/rstylesheet_pack_tag.txt' \
 -i app/views/layouts/application.html.erb

$s1 is only used once, and doesn't add any value (such as a meaningful name), so just inline it.

If we can eliminate the one use of C-escape quoting, then we can use plain POSIX shell instead of Bash.


#Modified code #!/bin/sh

set -eu
yarn add bootstrap jquery popper.js expose-loader
yarn install --check-files
sed -e '/^extract_css:/:.*/ true/' \
 -i config/webpacker.yml
printf '\n\n%s' 'import "bootstrap/dist/js/bootstrap";' \
 >> app/javascript/packs/application.js
echo '@import "~bootstrap/scss/bootstrap";' \
 > app/javascript/packs/styles.scss
sed -e '/module.exports = environment/rexpose-loader.txt' \
 -i config/webpack/environment.js
sed -e '/javascript_pack_tag .application/rstylesheet_pack_tag.txt' \
 -i app/views/layouts/application.html.erb

It's worth starting with a shebang; I'd also advise setting shell flags -e (exit on error) and -u (error when using unset variables):

#!/bin/bash
set -eu

Please use more whitespace and comments to make it easier to follow.

Perl is overkill for the simple substitution here - if we stick to sed, then that's one fewer dependency:

sed -e '/^extract_css:/:.*/ true/' \
 -i config/webpacker.yml

I omitted creating the backup because I don't think there's any value in keeping the original. If your later processes depend on having it around, then use -i.bak instead of -i, as in Perl (in fact, that's where Perl got that option).

The two transform-and-move pairs can also use sed -i, making the script more consistent, and thus easier to follow. We can use the (standard sed) r command to read a file, instead of the non-standard e command:

sed -e '/module.exports = environment/rexpose-loader.txt' \
 -i config/webpack/environment.js
sed -e '/javascript_pack_tag .application/rstylesheet_pack_tag.txt' \
 -i app/views/layouts/application.html.erb

$s1 is only used once, and doesn't add any value (such as a meaningful name), so just inline it.

If we can eliminate the one use of C-escape quoting, then we can use plain POSIX shell instead of Bash.


Modified code

#!/bin/sh
set -eu
yarn add bootstrap jquery popper.js expose-loader
yarn install --check-files
sed -e '/^extract_css:/:.*/ true/' \
 -i config/webpacker.yml
printf '\n\n%s' 'import "bootstrap/dist/js/bootstrap";' \
 >> app/javascript/packs/application.js
echo '@import "~bootstrap/scss/bootstrap";' \
 > app/javascript/packs/styles.scss
sed -e '/module.exports = environment/rexpose-loader.txt' \
 -i config/webpack/environment.js
sed -e '/javascript_pack_tag .application/rstylesheet_pack_tag.txt' \
 -i app/views/layouts/application.html.erb
I don't think the backups are necessary
Source Link
Toby Speight
  • 87.9k
  • 14
  • 104
  • 325

It's worth starting with a shebang; I'd also advise setting shell flags -e (exit on error) and -u (error when using unset variables):

#!/bin/bash
set -eu

Please use more whitespace and comments to make it easier to follow.

Perl is overkill for the simple substitution here - if we stick to sed, then that's one fewer dependency:

sed -e '/^extract_css:/:.*/ true/' \
 -i config/webpacker.yml

I omitted creating the backup because I don't think there's any value in keeping the original. If your later processes depend on having it around, then use -i.bak instead of -i, as in Perl (in fact, that's where Perl got that option).

The two transform-and-move pairs can also use sed -i, making the script more consistent, and thus easier to follow. We can use the (standard sed) r command to read a file, instead of the non-standard e command:

sed -e '/module.exports = environment/rexpose-loader.txt' \
 -i config/webpack/environment.js
sed -e '/javascript_pack_tag .application/rstylesheet_pack_tag.txt' \
 -i app/views/layouts/application.html.erb

$s1 is only used once, and doesn't add any value (such as a meaningful name), so just inline it.

If we can eliminate the one use of C-escape quoting, then we can use plain POSIX shell instead of Bash.


#Modified code #!/bin/sh

set -eu
yarn add bootstrap jquery popper.js expose-loader
yarn install --check-files
sed -e '/^extract_css:/:.*/ true/' \
 -i config/webpacker.yml
printf '\n\n%s' 'import "bootstrap/dist/js/bootstrap";' \
 >> app/javascript/packs/application.js
echo '@import "~bootstrap/scss/bootstrap";' \
 > app/javascript/packs/styles.scss
sed -e '/module.exports = environment/rexpose-loader.txt' \
 -i config/webpack/environment.js
sed -e '/javascript_pack_tag .application/rstylesheet_pack_tag.txt' \
 -i app/views/layouts/application.html.erb

It's worth starting with a shebang; I'd also advise setting shell flags -e (exit on error) and -u (error when using unset variables):

#!/bin/bash
set -eu

Please use more whitespace and comments to make it easier to follow.

Perl is overkill for the simple substitution here - if we stick to sed, then that's one fewer dependency:

sed -e '/^extract_css:/:.*/ true/' \
 -i config/webpacker.yml

The two transform-and-move pairs can also use sed -i, making the script more consistent, and thus easier to follow. We can use the (standard sed) r command to read a file, instead of the non-standard e command:

sed -e '/module.exports = environment/rexpose-loader.txt' \
 -i config/webpack/environment.js
sed -e '/javascript_pack_tag .application/rstylesheet_pack_tag.txt' \
 -i app/views/layouts/application.html.erb

$s1 is only used once, and doesn't add any value (such as a meaningful name), so just inline it.

If we can eliminate the one use of C-escape quoting, then we can use plain POSIX shell instead of Bash.


#Modified code #!/bin/sh

set -eu
yarn add bootstrap jquery popper.js expose-loader
yarn install --check-files
sed -e '/^extract_css:/:.*/ true/' \
 -i config/webpacker.yml
printf '\n\n%s' 'import "bootstrap/dist/js/bootstrap";' \
 >> app/javascript/packs/application.js
echo '@import "~bootstrap/scss/bootstrap";' \
 > app/javascript/packs/styles.scss
sed -e '/module.exports = environment/rexpose-loader.txt' \
 -i config/webpack/environment.js
sed -e '/javascript_pack_tag .application/rstylesheet_pack_tag.txt' \
 -i app/views/layouts/application.html.erb

It's worth starting with a shebang; I'd also advise setting shell flags -e (exit on error) and -u (error when using unset variables):

#!/bin/bash
set -eu

Please use more whitespace and comments to make it easier to follow.

Perl is overkill for the simple substitution here - if we stick to sed, then that's one fewer dependency:

sed -e '/^extract_css:/:.*/ true/' \
 -i config/webpacker.yml

I omitted creating the backup because I don't think there's any value in keeping the original. If your later processes depend on having it around, then use -i.bak instead of -i, as in Perl (in fact, that's where Perl got that option).

The two transform-and-move pairs can also use sed -i, making the script more consistent, and thus easier to follow. We can use the (standard sed) r command to read a file, instead of the non-standard e command:

sed -e '/module.exports = environment/rexpose-loader.txt' \
 -i config/webpack/environment.js
sed -e '/javascript_pack_tag .application/rstylesheet_pack_tag.txt' \
 -i app/views/layouts/application.html.erb

$s1 is only used once, and doesn't add any value (such as a meaningful name), so just inline it.

If we can eliminate the one use of C-escape quoting, then we can use plain POSIX shell instead of Bash.


#Modified code #!/bin/sh

set -eu
yarn add bootstrap jquery popper.js expose-loader
yarn install --check-files
sed -e '/^extract_css:/:.*/ true/' \
 -i config/webpacker.yml
printf '\n\n%s' 'import "bootstrap/dist/js/bootstrap";' \
 >> app/javascript/packs/application.js
echo '@import "~bootstrap/scss/bootstrap";' \
 > app/javascript/packs/styles.scss
sed -e '/module.exports = environment/rexpose-loader.txt' \
 -i config/webpack/environment.js
sed -e '/javascript_pack_tag .application/rstylesheet_pack_tag.txt' \
 -i app/views/layouts/application.html.erb
Source Link
Toby Speight
  • 87.9k
  • 14
  • 104
  • 325

It's worth starting with a shebang; I'd also advise setting shell flags -e (exit on error) and -u (error when using unset variables):

#!/bin/bash
set -eu

Please use more whitespace and comments to make it easier to follow.

Perl is overkill for the simple substitution here - if we stick to sed, then that's one fewer dependency:

sed -e '/^extract_css:/:.*/ true/' \
 -i config/webpacker.yml

The two transform-and-move pairs can also use sed -i, making the script more consistent, and thus easier to follow. We can use the (standard sed) r command to read a file, instead of the non-standard e command:

sed -e '/module.exports = environment/rexpose-loader.txt' \
 -i config/webpack/environment.js
sed -e '/javascript_pack_tag .application/rstylesheet_pack_tag.txt' \
 -i app/views/layouts/application.html.erb

$s1 is only used once, and doesn't add any value (such as a meaningful name), so just inline it.

If we can eliminate the one use of C-escape quoting, then we can use plain POSIX shell instead of Bash.


#Modified code #!/bin/sh

set -eu
yarn add bootstrap jquery popper.js expose-loader
yarn install --check-files
sed -e '/^extract_css:/:.*/ true/' \
 -i config/webpacker.yml
printf '\n\n%s' 'import "bootstrap/dist/js/bootstrap";' \
 >> app/javascript/packs/application.js
echo '@import "~bootstrap/scss/bootstrap";' \
 > app/javascript/packs/styles.scss
sed -e '/module.exports = environment/rexpose-loader.txt' \
 -i config/webpack/environment.js
sed -e '/javascript_pack_tag .application/rstylesheet_pack_tag.txt' \
 -i app/views/layouts/application.html.erb
lang-bash

AltStyle によって変換されたページ (->オリジナル) /