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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Figma API token optional in export script, fix CI issues #234

Merged
merged 29 commits into from
Aug 28, 2018

Conversation

shawnbot
Copy link
Contributor

This is intended to fix #233 by working around the absence of the FIGMA_TOKEN environment variable on Travis. Here's how it works:

  • If there is a FIGMA_TOKEN, we use that and fetch the icons from Figma as usual.
  • Otherwise, we:
    1. Get the version field from lib/octicons_node/package.json
    2. Fetch the top-level package.json from GitHub at the versioned tag (https://raw.githubusercontent.com/primer/octicons/v${version}/package.json)
    3. If the figma.url field is the same as the one in the local top-level package.json, we can assume that the icons haven't changed in this build and fetch the build files from https://unpkg.com/octicons@${version}/build/
    4. Otherwise, we die. 馃拃

One issue that I ran into getting files from unpkg.com was a mysterious SSL/TLS error complaining about a self-signed cert. I think it's the same as this error, but none of the suggested solutions in that thread worked for me, so I worked around the Node's TLS by shelling out to curl -sL. 炉\_馃_/炉

@shawnbot shawnbot requested a review from jonrohan August 28, 2018 17:03
@shawnbot shawnbot mentioned this pull request Aug 28, 2018
6 tasks
@shawnbot shawnbot changed the base branch from master to release-8.0.1 August 28, 2018 20:19
.gitignore Outdated
build/
lib/octicons_gem/lib/data.json
lib/build/
lib/**/build/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These lines are redundant.

@@ -5,6 +5,7 @@
"scripts": {
"version": "../../script/rubyversion ./lib/octicons_helper/version.rb",
"postinstall": "bundle install --path vendor/bundle",
"prepare": "echo '(no prepare needed here)'",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This makes it easier to just run npx lerna run prepare from the top level.

gem "rubocop"
gem "rubocop-github"
gem "rubocop", "0.50"
gem "rubocop-github", "0.5.0"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated these to match the versions in octicons_gem/Gemfile.

"ts-test": "tsc -P ts-tests",
"test": "jest",
"posttest": "npm run ts-test",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TypeScript tests are run after the main test suite now.

@shawnbot shawnbot changed the title Make Figma API token optional in export script Make Figma API token optional in export script, fix CI issues Aug 28, 2018
@shawnbot
Copy link
Contributor Author

FYI @jonrohan, our RUBYGEMS_USER environment variable was hidden in Travis, which is why previous builds had output that included strings like @github[secure]/octicons-react. I updated that variable to be displayed in the build logs so that it's easier to debug these package publish errors.

@@ -3,7 +3,7 @@ set -e
# pwd
package=$(jq -r .name package.json)
version=$(jq -r .version package.json)
published=$(npm info $package@$version version)
published=$(npm info "$package@$version" version || echo "0.0.0")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This used to throw an error if the package you specified didn't exist. If that's the case, just return 0.0.0.

@shawnbot shawnbot changed the base branch from release-8.0.1 to dev August 28, 2018 21:08
@shawnbot shawnbot merged commit d82606b into dev Aug 28, 2018
@shawnbot shawnbot deleted the optional-figma-token branch August 28, 2018 21:09
This was referenced Aug 28, 2018
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.

None yet

2 participants