ci: allow multi submission
Double check these details before you open a PR
- [x] PR does not match another non-stale PR currently opened
Features
This PR allows multi submission edits in a single PR, as discussed in https://github.com/devicons/devicon/pull/2270.
This PR closes NONE
Notes
I have tested this against the same branch that creates https://github.com/devicons/devicon/pull/2270 in my local fork. This is the resulting comment: https://github.com/ReenigneArcher/devicon/pull/1#issuecomment-2456068176. All of the comments are valid issues, so I believe this is working as expected.
Note, that somewhere in the code empty strings are getting added to the error list. I don't believe this is anything from my changes, but I have just removed the empties using the following line.
err_msg = list(filter(None, err_msg)) # remove empty strings from err_msg
Some trailing whitespace was also automatically removed from lines in modified files.
@Snailedlt FYI
Great! I'll see if I can take a look at the PR this weekend
However I think we might get issues with the release script, since that also builds icons based on the PR title.
Thanks for the review. I'll dig into that in the next few days and report back.
Just pasting this code snip here so I can easily find it when I go to finish this:
https://github.com/devicons/devicon/blob/ca28c779441053191ff11710fe24a9e6c23690d6/.github/scripts/icomoon_build.py#L72-101
I actually believe this may already work as is due to this section:
# get any icons that might not have been found by the API
# sometimes happen due to the PR being opened before the latest build release
new_icons_from_devicon_json = filehandler.find_new_icons_in_devicon_json(
devicon_json_path, icomoon_json_path)
@Snailedlt I'm not sure I can complete a release test in my fork.
For some reason the build_icons.yml workflow is not appearing under the actions tab of my repo.
https://github.com/ReenigneArcher/devicon/actions
I've only ever seen this when the workflow file does not exist on the default branch, but that is not the case here. Any idea?
@ReenigneArcher hmmm, that's very odd. No idea sadly... but maybe it helps to recreate the repo instead of forking it? You could also try re-forking it
I re-forked the repo into another org, and that one seems to be fine. I opened a ticket with GitHub support, hopefully they can solve it as I'd prefer not to delete my current fork.
@Snailedlt I got it to appear by making a small adjustment to the workflow (no functional change)... but the workflow fails due to deprecated actions. I will make a PR updating the deprecated actions, and I can also introduce dependabot which can automatically keep workflow actions up to date by creating PRs when new versions are released. Here's an example PR: https://github.com/LizardByte/Sunshine/pull/3399
PR to update actions: https://github.com/devicons/devicon/pull/2310
@Snailedlt could you take a look at #2310? This PR depends upon that one, and actually any future workflow runs in this repo as many of these outdated actions are going to stop working altogether (as they are automatically failing workflows): https://github.blog/changelog/2024-02-13-deprecation-notice-v1-and-v2-of-the-artifact-actions/
@ReenigneArcher Now that #2310 has been merged, this PR should be possible to test, right?
@Snailedlt yes, I believe so. I just need to re-gather my thoughts on this as it has been a a while since I looked at all this.
I started looking at this again tonight. I found some things I missed which is bring up some questions.
- This will affect the
icomoon_peek.pyfile as well. I checked the action logs and this workflow is ALWAYS skipped. https://github.com/devicons/devicon/actions/workflows/peek_icons.yml ... Can I remove the workflow and the script? I'm not sure the history on this, but looks like this workflow is intended to run when thebot:peeklabel is added which hasn't been used since May 2023. https://github.com/devicons/devicon/pulls?q=is%3Apr+label%3Abot%3Apeek+is%3Aclosed - For some reason I am getting an exit code of 1, with nothing printed. To me this indicates it is even failing before this line: https://github.com/ReenigneArcher/devicon/blob/9436f2555dfe3e1626758394786789a05f507927/.github/scripts/icomoon_build.py#L27 ... Possibly in the args getter. I've used
argparsequite a bit, but always with named arguments, never positional. Normally when argparse fails, it's prints out a "usage" statement, so I'm still not sure if that's it. Any ideas?
I have a commit in the testing branch which doesn't exit in my PR (I will bring everything in once I get it working): https://github.com/ReenigneArcher/devicon/commits/draft-release/
- The bot peek script stopped working a long time ago. I believe because of an PAI change, or some bandwidth limit in the API. When it worked it would post a few screenshots of how the icon font looks when uploaded to icomoon. It would be awesome if we got it working again, since it's a hazzle to upload and check the font manually... But it's not required to get this PR merged. So feel free to try and fix it, or delete it. Whichever you want :)
- I've got no idea sadly. Logging the arguments might be a good place to start though. I think you're right about it exiting before that line, maybe even before that method is called. Maybe you can try changing to named arguments, since that makes it quite a bit easier to read.
- With that information, I will keep the script and adjust to accept the potentially multiple changed icons in a PR. I'll save trying to get the script actually working again for a later PR though.
- I may make a separate PR to start using named arguments, just so this PR doesn't become too overwhelming to review.
I figured out why there was no output. It seems the output is redirected to a log file which is then uploaded as an artifact.
Also, I think I am making some progress, but getting at error at the end of the build.
I don't think this error has anything to do with my changes, but I might be mistaken on that.
Finished uploading all files.
Finished uploading the svgs...
Taking screenshot of the new icons...
Saved screenshot of the new icons...
Downloading Font files...
Font files downloaded.
Extracting zipped files...
it's zipped True
Traceback (most recent call last):
File "/home/runner/work/devicon/devicon/./.github/scripts/icomoon_build.py", line 49, in main
filehandler.extract_files(str(zip_path), args.download_path, logfile)
File "/home/runner/work/devicon/devicon/.github/scripts/build_assets/filehandler.py", line 158, in extract_files
icomoon_zip.extract(file, extract_path)
File "/opt/hostedtoolcache/Python/3.10.16/x64/lib/python3.10/zipfile.py", line 1643, in extract
return self._extract_member(member, path, pwd)
File "/opt/hostedtoolcache/Python/3.10.16/x64/lib/python3.10/zipfile.py", line 1682, in _extract_member
member = self.getinfo(member)
File "/opt/hostedtoolcache/Python/3.10.16/x64/lib/python3.10/zipfile.py", line 1452, in getinfo
raise KeyError(
KeyError: "There is no item named 'fonts/devicon.eot' in the archive"
Exiting
Closing down SeleniumRunner...
Failed job: https://github.com/ReenigneArcher/devicon/actions/runs/13318304932
Edit: To verify I'm running the build based strictly on the current develop branch. I expect to see the same error. https://github.com/ReenigneArcher/devicon/actions/runs/13299083430
Edit 2: Confirming that the error is the same. How should we handle this?
I figured out why there was no output. It seems the output is redirected to a log file which is then uploaded as an artifact.
Right, iirc it's uploaded as an artifact, and then later on the artifact is retrieved. It's a weird workaround for a very odd security issue with allowing workflows to run on forked repos. Here's an explanation of why we need to upload the artifact in a preflight script, and then download it again to use it: https://github.com/devicons/devicon/issues/1099
This is also related: https://github.com/devicons/devicon/issues/1715
KeyError: "There is no item named 'fonts/devicon.eot' in the archive"
Hmm, seems there is something wrong with the font creation in icomoon. devicon.eot is a file that should be downloaded from icomoon when creating the font. Maybe you could try creating the font manually.
I also recommend reading about how we build the font in our wiki to get a better grasp of how it all works. It's been a while since I read it myself, so maybe I should too 😅 Anyways, here are some links for you:
- https://github.com/devicons/devicon/wiki/Release-strategy,-conventions,-preparation-and-execution
- https://github.com/devicons/devicon/wiki/How-We-Create-The-Icons
- https://github.com/devicons/devicon/tree/develop?tab=readme-ov-file#build-the-new-icons
This wiki page is also very useful for an overview on most of our automated workflows, and it should also be updated when we update this one: https://github.com/devicons/devicon/wiki/Our-automated-tasks-and-bots
Hmm, it might also be that icomoon has renamed the .eot file to something else, or that the GitHub cache is stuck or something. I would suggest trying these two things first:
- Delete the cache for the workflow, and rerun it to see if that fixes anything
- Do the font creation process manually, to better understand how it works, and to see if the filenames and locations are as expected.
Hmm, it might also be that icomoon has renamed the .eot file
This was my thought as well, or that it was removed altogether. I'm not too versed in font creation.
I don't have any caches in my fork, so I don't think that is the issue. I will start with printing out all the contents of the archive, and reading the above references.
Sounds like a good plan. Let me know if you get stuck :)
Here are my findings so far:
- The file is not in the downloaded zip. These are the contents:
['selection.json', 'fonts/', 'fonts/devicon.ttf', 'fonts/devicon.woff', 'fonts/devicon.svg', 'demo.html', 'demo-files/', 'demo-files/demo.js', 'demo-files/demo.css', 'Read Me.txt', 'style.css']
I manually tried to create a font via the icomoon website (only with one of the default icons), downloaded the archive and it did contain the .eot file.
I also edited the workflow to upload the zip as an artifact:
My first thought was the archive was not fully downloaded; however I doubled the wait delay and the result was identical so this theory is probably not correct.
I'm not sure what else to check/try on this.
Hmm, that's interesting 🤔
So I'm guessing the issue is somewhere in the build_icons.yml script then. Either that or icomoon changes their API.
Have you checked if the .eot file is created when doing it on the master branch?
If it is, then there must be something we changed after the last release that broke it, and if not it's likely something external, because I don't remember having any issues with a missing .eot file when making the last release
Unfortunately, the workflows in master do not work due to the EOL GitHub actions. But:
- Nothing in scripts could be causing this as they haven't changed since the last release (other than a non breaking update to
requests): https://github.com/devicons/devicon/tree/develop/.github/scripts - Looking at the workflow changes, I don't see any changes since the last release either (other than action updates which wouldn't cause this)
I found the issue with the missing .eot file. The fix is in https://github.com/devicons/devicon/pull/2361