mozilla / FirefoxColor

Theming demo for Firefox Quantum and beyond
https://color.firefox.com
Mozilla Public License 2.0
461 stars 95 forks source link

potential fix for dev deploys / static test page #912

Closed rebmullin closed 4 years ago

rebmullin commented 4 years ago

hey @Rob--W,

I am still plan on working on #845 (I'll post my questions for that over there) but in the interim please let me know if this will help.

I am wasn't 100% sure about updating the webpack.common file tho. I believe the config.yml will override the value there so if that's the case (which we want for prod), then this should be okay.

NOTE: this reverts some of what is added here: https://github.com/mozilla/FirefoxColor/pull/842

rebmullin commented 4 years ago

hey @Rob--W,

I decided to add the work for the static page here since it's all related.

I am not 100% the new dev script is correct. I believe I have it doing the same same steps as stage/prod to create an unsigned addon - can you please confirm?

Note: I'll clean up the instructions once this is updated :)

rebmullin commented 4 years ago

hey @Rob--W, I followed some links noted in the other issue. I'm still not 100% that I have this right, so please let me know.

rebmullin commented 4 years ago

elif [ "${CIRCLE_BRANCH}" == "production" ] then SITE_URL="https://color.firefox.com/" ADDON_URL="https://addons.mozilla.org/firefox/addon/firefox-color/" else echo "Unknown branch: ${CIRCLE_BRANCH}" >&2 exit 1 fi

If I use the "else" as suggested above ^, it hits the "Unknown branch..." error here:

It seems that the branch is the pull request branch so it fails..

if we need a default /else maybe we can just do something like ~

...
elif [ "${CIRCLE_BRANCH}" == "stage" ]
then
  SITE_URL="https://color.stage.mozaws.net/"
  ADDON_URL="testing.html"
else
  SITE_URL="https://color.firefox.com/"
  ADDON_URL="https://addons.mozilla.org/firefox/addon/firefox-color/"
fi

thoughts?

Rob--W commented 4 years ago

If I use the "else" as suggested above ^, it hits the "Unknown branch..." error here:

It seems that the branch is the pull request branch so it fails..

That is somewhat expected; the current branch is pull/912 (and the target branch master, but I don't know if there is a variable for it).

When built from a pull request, the value of SITE_URL and ADDON_URL are not really important. To have a sensible default, let's not set any environment variables at all for that case, and print the current branch like we do right now (+ the reason for not setting variables).

Note: in the current PR you are not exporting the variables like I requested. For it to work correctly, you must include the cat <<... that I showed in my snippet. Please follow the format of my previous comment (without the exit 0).

rebmullin commented 4 years ago

To have a sensible default, let's not set any environment variables at all for that case, and print the current branch like we do right now (+ the reason for not setting variables).

okay so I left the "Unknown branch.." without the error exit code so it doesn't fail in circle ci.

Note: in the current PR you are not exporting the variables like I requested. For it to work correctly, you must include the cat <<... that I showed in my snippet. Please follow the format of my previous comment (without the exit 0).

woops! yep i missed that.

How's it look now? : )

rebmullin commented 4 years ago

thanks @Rob--W for all the help with this one!