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

Created scripts to update the current devicon.json and generate css files based on the new json file #290

Merged
merged 11 commits into from
Sep 30, 2020

Conversation

Thomas-Boi
Copy link
Member

@Thomas-Boi Thomas-Boi commented Sep 14, 2020

Hello @amacado,

Here are the contents for this PR:

Summary:

  • The upgradeDev.js would create a newDevicon.json that has the color and aliases attribute
  • The gulp tasks updateCss and cleanUp are used to create the css files.
  • The updateCss task used Sass to help with creating the devicon-alias.css, concatenation, and minification. We can remove the other gulp tasks that did these jobs.
  • The min.css created by updateCss might look weird but it works.
  • These fonts don't have colors, font versions, or just bad naming: mocha, clojure, clojurescript, redux, yunohost.

The upgrade devicon.json script:

  • The script is called upgradeDev.js. This is what it does:
    • Parse the devicon.json, devicon-colors.css and the devicon-alias.css.
    • Add any colors and aliases to the correct object in the deviconJson in memory
    • Write the deviconJson to a new file called newDevicon.json.
  • After the run, you should see some logs and artifacts:
    • The script would generate these artifacts:
      • contentMap.json: created from devicon.css and maps a class name to the content property
      • aliasMap.json: created from devicon-alias.css and maps the content property to an array of aliases
      • colorsMap.json: created from devicon-colors.css and maps a class name to the color property
      • See line 16 for details.
    • The logs would list font objects that didn't have a font version (aka no aliases) or a color attribute. See line 137 to see the filter.
  • Note:
    • dot-net in the newDevicon.json generated by the script doesn't have a color. This is due to the regexp that I used. It's faster if we just add the colors in manually afterwards, which is what I did for the newDevicon.json`.

The css-generating scripts:

  • They are located in the gulpfile.js.
  • The script utilized the Sass compiler, which is a CSS pre-processor.
  • What the script does:
    • It loops through the devicon.json or in this case, the newDevicon.json.
    • To generate the devicon-colors.css, it generates css as read from the deviconJson. It skips any objects that doesn't have a color or fonts.
    • To generate the devicon-alias.css, it generates css for the aliases using the @extend syntax from Sass. Whenever a selector @extend another selector, Sass knows to generate the same css for both of them. You can see the result in the min.css.
    • After creating these files, the script create min.scss where it imports the above files. I then use gulp to process this file, creating the min.css file (the file extension .scss means a Sass file).
  • Notes:
    • Sass concatenates and minifies the final css file. To see a minify version, go to line 37 and follow the instructions there. This means that we can remove the gulp-concat and gulp-minify dependencies.
    • The final output of min.css has the content property as squares (see below).

image

  • This is fine and doesn't create a problem (see this). You can test it by:

    • Open the index.html located at the root dir.
    • Find the <link> that imports the devicon.min.css
    • Change the href to min.css
    • You should see that the fonts are still there with no issues (disable cache before doing this).
  • You can run this gulp task by typing npm run build. For this task, I chain gulp tasks together rather than using the gulp.series(). This is because of a weird bug:

    • In the gulpfile.js, there's a task at the bottom called test which uses gulp.series(). Try running this task.
    • The task would run well; the min.css is generated correctly and the cleanUp() removed all artifacts.
    • However, gulp would log an error complaining that the min.scss file is not found. The source of this is from a package called duplexify, which might be from gulp or sass. In the cleanUp() function, I have tried changing unlink() to access() and things work fine (no errors).
    • I think there might be some async issues. However, I don't think that we should spend time on it considering the gulp.series() is only faster than && chaining by a few seconds.

Let me know what you think. Also, I'd need to clean up the branch before we merge it.

@Thomas-Boi Thomas-Boi added devops Use this label for devops related enhancements enhancement labels Sep 14, 2020
Copy link
Member

@amacado amacado left a comment

Choose a reason for hiding this comment

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

Looks good to me @Thomas-Boi! :) I would suggest to clean up the artifacts, no longer required routines and change the build output to our known format "devicon.json" and "devicon.min.css" instead of your testing values.

I would also suggest that you provide a updated README.md on how to use the devicon.json in the future (and removing the alias.css and colors.css). Ok? :)

Maybe we should change the target branch to develop?

@amacado amacado changed the base branch from master to develop September 22, 2020 21:45
@Thomas-Boi
Copy link
Member Author

Hey @amacado,

Thanks for looking at my change! I will address all of your suggestions in the next commit to the branch. Unfortunately, I am little busy with school right now so I will hopefully get it in by the weekends. I will also publish our discussions last week so everyone can see what we talked about as well.

Cheers,

@Thomas-Boi
Copy link
Member Author

Hey @amacado,

I have removed all unnecessary files from the repo (test files, unneeded tasks) and rename the built files to devicon.json and devicon.min.css. I have also tested everything before I commit and the build task still works.

Regarding the README.md update and the required changes to the build_icons.yml, I will create another branch called upgrade-workflow and open a PR later.

Cheers,

@Thomas-Boi Thomas-Boi closed this Sep 30, 2020
@Thomas-Boi Thomas-Boi reopened this Sep 30, 2020
@Thomas-Boi Thomas-Boi merged commit 3bc1d9a into develop Sep 30, 2020
@amacado amacado linked an issue Oct 9, 2020 that may be closed by this pull request
@Thomas-Boi Thomas-Boi deleted the upgrade-devicon branch October 9, 2020 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devops Use this label for devops related enhancements enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

On the new workflow and pull request standards
2 participants