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

Make sure that the preview banner is shown when there is no web folder #1292

Merged
merged 2 commits into from
Feb 6, 2023

Conversation

matteodepalo
Copy link
Contributor

WHY are these changes introduced?

Fixes https://github.com/Shopify/internal-cli-foundations/issues/550

We weren't showing the footer with the preview url in dev if there was no web folder and only UI extensions

WHAT is this pull request doing?

This PR makes sure that the previewUrl is initialized outside the check for web or backend. Note that if we had function or theme extensions only, we would still not show the banner, but this is temporary while we're moving the instructions for both to the dev console.

How to test your changes?

  • Delete the web folder in the fixture app
  • Run dev
  • A footer with the dev console URL should appear

Measuring impact

How do we know this change was effective? Please choose one:

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
  • Existing analytics will cater for this addition
  • PR includes analytics changes to measure impact

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've made sure that any changes to dev or deploy have been reflected in the internal flowchart.

@matteodepalo matteodepalo requested a review from Arkham February 6, 2023 01:03
@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 6, 2023

Benchmark report

The following table contains a summary of the startup time for all commands.

Status Command Baseline (avg) Current (avg) Diff
🟢 app build 875 ms 869 ms -0.69 %
🟢 app deploy 1047.33 ms 1051 ms 0.35 %
🟢 app dev 1054.33 ms 1054.67 ms 0.03 %
🟢 app env pull 958 ms 967 ms 0.94 %
🟢 app env show 954.33 ms 980 ms 2.69 %
🟢 app generate extension 1027.33 ms 1011.67 ms -1.52 %
🟢 app generate schema 989.67 ms 988.33 ms -0.13 %
🟢 app info 961 ms 965 ms 0.42 %
🟢 app scaffold extension 1012.67 ms 1024.67 ms 1.18 %
🟢 app update-url 937 ms 941 ms 0.43 %
🟢 theme check 816.33 ms 823 ms 0.82 %
🟢 theme delete 921.33 ms 919 ms -0.25 %
🟢 theme dev 914.67 ms 912 ms -0.29 %
🟢 theme help-old 821.33 ms 815.67 ms -0.69 %
🟢 theme info 853.67 ms 853 ms -0.08 %
🟢 theme init 831.33 ms 833.67 ms 0.28 %
🟢 theme language-server 815.33 ms 817.33 ms 0.25 %
🟢 theme list 931 ms 910.67 ms -2.18 %
🟢 theme open 927 ms 932.67 ms 0.61 %
🟢 theme package 859.67 ms 856 ms -0.43 %
🟢 theme publish 925 ms 926.67 ms 0.18 %
🟢 theme pull 909.67 ms 923.33 ms 1.5 %
🟢 theme push 910.67 ms 915.67 ms 0.55 %
🟢 theme serve 917.67 ms 920.33 ms 0.29 %
🟢 theme share 916.67 ms 911 ms -0.62 %
🟢 webhook trigger 912.67 ms 922.67 ms 1.1 %

@github-actions
Copy link
Contributor

github-actions bot commented Feb 6, 2023

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
71.85% (+0.01% 🔼)
3843/5349
🟡 Branches 69.42% 1739/2505
🟡 Functions 70.17% 1002/1428
🟡 Lines
73.08% (+0.01% 🔼)
3665/5015

Test suite run success

971 tests passing in 500 suites.

Report generated by 🧪jest coverage report action from c729e47

@Arkham Arkham merged commit b11a50b into main Feb 6, 2023
@Arkham Arkham deleted the preview-banner-when-no-web branch February 6, 2023 19:38
@shopify-shipit shopify-shipit bot temporarily deployed to nightly February 7, 2023 02:14 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to production February 7, 2023 10:42 Inactive
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.

2 participants