Introducing github action for build automatisation (using python)

[Thomas-Boi]

Hello all,

I have finished the build script for devicon.

Here are the features that this branch has:

• The build script is a GitHub Action that will be triggered on push
• The script will perform all the tasks that were laid out in the issue discussion about the script. It will also check whether the zip file is downloaded before it continues.
• After the files are built, the workflow will use a commit action to commit the built files into the repo.

Currently, the new files are put into a folder called built_files. This is done so that reviewers can test the process without loosing the original copies. Before we merge, we'd need to change it so that the script will replace the old files with the new ones.

All feedback and discussions are appreciated.

[amacado]

I will take a closer look in the upcomming days! :) Been a bit busy now but it's on my todo list! :)

[amacado]

First of all and I can't say this to often: A very big thank you, i really appreciate your contribution in ideas and code. Sorry for the delay, but since I do this project in my free time and i'm currently pretty busy it took me some time to review and test it. Since this addition improves the quality of this project a lot I wanted to take a deep look into your contribution. Sidenote: Good comments in the python scripts! Helps a lot to figure out what happens! 👍🏻 I tested the github action by forking the project to my own.

Here are my suggestions/comments:

• Can the .pyc (py-cache) scripts be removed? I don't think they are required in the repo, or are they? (f.e. .github/scripts/build_assets/__pycache__/PathResolverAction.cpython-38.pyc). We should also add them to .gitignore, since they were added and commited by git git add . command running in the action.
• .github/scripts/build_assets/geckodriver.exe should be documented (version, source,..) maybe add a readme?
• As we figured out in our discussion it would be good to run our gulp task gulp default (for combining the colors/alias and creating a minified version) after finishing the iconmoon job. Maybe you could introduce this?
• Is there a reason to include the icon changes to haskell in this branch? I would suggest separate this to keep things clean.
• In your code-comments you often write icomoon_test.json and devicon_test.json is this intented?

Happy to hear your feedback on my review! :)

[Thomas-Boi]

Hey @amacado,

I really appreciate your feedback :)

I understand the delay and I am not bothered by it. In fact, I think I might be in the same boat as you in the next few months. I also would like to thank you for going over my codes and discussing things with me. It really helps having a second person do a code review on something like this.

So as always, I'll address your points one by one:

Can the .pyc (py-cache) scripts be removed? I don't think they are required in the repo, or are they? (f.e. .github/scripts/build_assets/pycache/PathResolverAction.cpython-38.pyc). We should also add them to .gitignore, since they were added and commited by git git add . command running in the action.

The cache is not essential so I can definitely do that. The IDE that I use would hide this folder while I work so I sometimes forget that it exists.

.github/scripts/build_assets/geckodriver.exe should be documented (version, source,..) maybe add a readme?

That sounds good to me. I'll add a README about it.

As we figured out in our discussion it would be good to run our gulp task gulp default (for combining the colors/alias and creating a minified version) after finishing the iconmoon job. Maybe you could introduce this?

This is on my TODO list. I'm currently working on adding the color and aliases attributes to the devicon.json. Once I'm done with it, I will start working on the gulp task and add it to our workflow.

Is there a reason to include the icon changes to haskell in this branch? I would suggest separate this to keep things clean.

Are you referring to the fact that the haskell folder is moved in to the fonts folder? If you are, I did it just to clean the directory up. I was confused why the folder stayed in the root directory for so long in the first place. If there's a reason for it, I can move it back.

In your code-comments you often write icomoon_test.json and devicon_test.json is this intented?

That was not intended. During testing, I would rename the original files to icomoon_test.json and devicon_test.json. My IDE would then automatically update all the references to these files, which included docs. I thought I fixed all of them so thanks for spotting it.

I hope that answered all your questions. I'll start working on your suggestions and I'll let you know when I'm done.

[amacado]

Are you referring to the fact that the haskell folder is moved in to the fonts folder? If you are, I did it just to clean the directory up. I was confused why the folder stayed in the root directory for so long in the first place. If there's a reason for it, I can move it back.

ah! Now I understand. But again, I would suggest to track this issue 274 in a separated pull request, I will open one to keep our directory structure clean and not mixin this with our build script.
Update: I fixed the haskell icon structure in #275 and merged the updated master back into this branch. Since I did not want to force push your branch I created a pull request to this branch including the current master branch merge: #276. This fill resolve the conflicts which are currently pointed out in this branch:

I hope that answered all your questions. I'll start working on your suggestions and I'll let you know when I'm done.

looking forward to review your changes! :) happy coding!

[amacado]

@Thomas-Boi well done! I did some intense reviewing and here are the results. I addressed most of them myself in #281 which can be merged into this build-integrate branch.

Those two icons contain a typo "workmark" instead "wordmark" (which a already resolved in the current master branch 493ebf3). I will create a pull request to remove them in this branch as well. Probably a merging error?

icons/haskell/haskell-original-workmark.svg
icons/haskell/haskell-plain-workmark.svg

Tried to address a bug where the icomoon_upload.py failed during build for the first time but run perfectly for the next times.

While testing I encountered the same bug. The first time the action runs it will throw the following action. The second re-run has thrown the same error. In the third try it worked as expected. I did not investigate further but throughout my testing this behaviour encountered multiple times.

Run python ./.github/scripts/icomoon_upload.py  ./.github/scripts/build_assets/geckodriver-v0.27.0-win64/geckodriver.exe ./icomoon.json ./devicon.json ./icons ./ --headless
Message: No connection could be made because the target machine actively refused it. (os error 10061)

None
Traceback (most recent call last):
  File "./.github/scripts/icomoon_upload.py", line 47, in main
    runner = SeleniumRunner(args.icomoon_json_path, args.download_path,
  File "D:\a\devicon\devicon\.github\scripts\build_assets\SeleniumRunner.py", line 52, in __init__
    self.set_options(geckodriver_path, headless)
  File "D:\a\devicon\devicon\.github\scripts\build_assets\SeleniumRunner.py", line 75, in set_options
    self.driver = WebDriver(options=options, executable_path=geckodriver_path)
  File "C:\hostedtoolcache\windows\Python\3.8.5\x64\lib\site-packages\selenium\webdriver\firefox\webdriver.py", line 170, in __init__
    RemoteWebDriver.__init__(
  File "C:\hostedtoolcache\windows\Python\3.8.5\x64\lib\site-packages\selenium\webdriver\remote\webdriver.py", line 157, in __init__
    self.start_session(capabilities, browser_profile)
  File "C:\hostedtoolcache\windows\Python\3.8.5\x64\lib\site-packages\selenium\webdriver\remote\webdriver.py", line 252, in start_session
    response = self.execute(Command.NEW_SESSION, parameters)
  File "C:\hostedtoolcache\windows\Python\3.8.5\x64\lib\site-packages\selenium\webdriver\remote\webdriver.py", line 321, in execute
    self.error_handler.check_response(response)
  File "C:\hostedtoolcache\windows\Python\3.8.5\x64\lib\site-packages\selenium\webdriver\remote\errorhandler.py", line 242, in check_response
    raise exception_class(message, screen, stacktrace)
selenium.common.exceptions.TimeoutException: Message: No connection could be made because the target machine actively refused it. (os error 10061)

Added a upload-artifact action to upload the geckodriver.log for debugging purpose

I like that! 👍🏻 I propose to setup a name for the artifact, will add this in my pull request.

Change the extract_path to the root directory. From now on, the script will replace the icomoon.json and the font folder after successful runs.

So far so good, I will remove the built_files since it's no longer used then.

The commit action required a branch input to function properly for PR trigger => Added that in the workflow yml.

As in the documentation pointed out when no branch is given as parameter it will use the current branch as target (which is what we want). When you hard-code build-integrate at this place the action failes after the pull request is merged into another branch (f.e. master).

[amacado]

In my opinion after #281 is beeing merged this pull request is fine for the first working version of our build automatisation. If @Thomas-Boi approves this can be merged into master. The next (independent) step would be the transformation of devicon.json to the new format including color and alias where we can replace the current gulp default task (@Thomas-Boi started working on it in #279).

interface NewDeviconObject extends DeviconObject {
  "color": string;  // color for the colored version
  "alias": [
                  {
                       "base": string;
                       "alias": string;
                  },
                  {
                       "base": string;
                       "alias": string;
                  }
              ]
}

[Thomas-Boi]

I am fine with the changes that you introduced. However, there's one small change: the alias attribute would be aliases instead. This is only a semantic change and would not affect functionality.

interface NewDeviconObject extends DeviconObject {
  "color": string;  // color for the colored version
  "aliases": [
                  {
                       "base": string;
                       "alias": string;
                  },
                  {
                       "base": string;
                       "alias": string;
                  }
              ]
}