Skip to content
This repository has been archived by the owner on Dec 20, 2018. It is now read-only.

Commit

Permalink
Remove phantomjs dependency, update git add deploy command, v0.1.24
Browse files Browse the repository at this point in the history
  • Loading branch information
Kamil Nikel committed Dec 28, 2015
1 parent 57db2d1 commit 8e54d03
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 4 deletions.
2 changes: 1 addition & 1 deletion .travis.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ after_success: |
printf "\nCommitting and pushing to release repo..."
cd ../traffico-release
git add .
git add --all .
git commit -m "Drop in $TRAVIS_TAG"
git tag "$TRAVIS_TAG"
git push origin master "$TRAVIS_TAG"
Expand Down
4 changes: 1 addition & 3 deletions package.json
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "traffico",
"version": "0.1.23",
"version": "0.1.24",
"description": "Traffic signs packaged in a font",
"main": "gulpfile.js",
"repository": {
Expand All @@ -22,8 +22,6 @@
"gulp-shell": "^0.5.1",
"gulp-watch": "^4.3.5",
"gulp-zip": "3.0.2",
"phantom": "^0.8.4",
"phantomjs": "^1.9.19",

This comment has been minimized.

Sign in to view

Copy link
@floscher

floscher Dec 29, 2015

Contributor

Hi @knikel,
the phantom dependency is not used by the standard build (npm run build or npm run check), but it is needed for png generation (npm run pngs). See

traffico/gulpfile.js

Line 15 in 8e54d03

.pipe(shell(['phantomjs <%= file.path %>']))
.
This was introduced not very long ago in fb4494d by @peterneubauer.
"standard": "^5.4.1"
},
"scripts": {
Expand Down

3 comments on commit 8e54d03

@xeob
Copy link
Contributor

@xeob xeob commented on 8e54d03 Dec 29, 2015

Choose a reason for hiding this comment

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

@floscher That's correct, but we're not using the png generation and it needs a refactor. I'd like to refactor that and possibly find a way to remove phantomjs dependency. The reason I removed it was timing out and erroring 50% of Travis CI builds. That was a conscious choice here 😄. I'll move the png generation off the main branch, as this is not fully developed feature.

@floscher
Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, then it's fine. I just thought you wanted to remove unused dependencies and overlooked the use of phantom.
But it's strange that it led to failing Travis builds, as Travis only runs the check task, or did the dependency not install correctly?
Regarding png-generation, maybe svg2png or gulp-svg2png might be worth a try.
And another task that I recently found is mostly unnecessary, is patch-names. It currently only patches one single sign name, as the others have changed names since the task was introduced.

@xeob
Copy link
Contributor

@xeob xeob commented on 8e54d03 Dec 29, 2015

Choose a reason for hiding this comment

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

@floscher The phantomjs binary is quite big, and bitbucket where it's fetched from was timing out. It was especially the case after two consecutive releases which I did yesterday. I had to manually rerun Travis CI builds from the Travis UI three times. So yes, the package install part was flopping.

Thanks for reminder about that task, it should be removed, added it to my to do list. This task is also something very Mapillary specific, as we used it for keeping traffic sign names across detection packages in sync. Since then a lot has changed. Nowadays if someone would like to use traffico in context of Mapillary data, then mapillary-mappings is the way to patch names. In the ideal scenario, we should be serving patched names from the API without needing to patch them on the client.

Please sign in to comment.