Closed Thomas-Boi closed 4 years ago
Thank you for you contribution @Thomas-Boi, I had a look at your scripts (haven't executed them yet) and I'm having some questions/thoughts about it:
input.json
needs to be modified with the 'new' icons which I want to add to the collection. In my opinion this is not very intuitive, since this would lead into an additional file (input.json
) which needs to be managed as well. I would prefer a solution where we need to only maintain one file. Maybe we could use our existing devicon.json
for that? We may have to rename some of the icons/*
folders but this would be a trade-off I'm willing to accept. In this case we also need a function which is managing the state of the current iconmoon.json (so we do not add icons twice).devicon-colors.css
? Our existing gulp task
is already in charge of combining the resulting css files from iconmoon, the colors and alias. See gulpfile.js#L7. Since you correctly pointed out that using complex icons in iconmoon app isn't a good idea I would prefer to stick to our existing solution for that.build action
once a new pull request is merged into master. Again: Thanks for your time spend improving this project :heart:
Hey @amacado,
Appreciate the feedback! I'll address your questions point by point:
If i got it right, then the script is used in a way where the input.json needs to be modified with the 'new' icons which I want to add to the collection. In my opinion this is not very intuitive, since this would lead into an additional file (input.json) which needs to be managed as well. I would prefer a solution where we need to only maintain one file. Maybe we could use our existing devicon.json for that? We may have to rename some of the icons/ folders but this would be a trade-off I'm willing to accept.
devicon.json
for the build script. I was a new contributor at the time and I didn't want to drastically change it (since the website uses that to load the icons). However, I agree that it's better to have less files in the repo. How would you feel if we change devicon.json
to something like this:json
{
"new": [
// content of `input.json`
],
"processed": [
// current content of `devicon.json`
]
}
After the script finish the task, it can splice the "new" array into the "processed" array then write it to the devicon.json.
In this case we also need a function which is managing the state of the current iconmoon.json (so we do not add icons twice).
selection.json
. We can rename it to "icomoon.json" when it extracts the file from the zip (this is on my todo list).Same thoughts about the colors, why managing an additonal settings file for that when we already have the colors in devicon-colors.css? Our existing gulp task is already in charge of combining the resulting css files from iconmoon, the colors and alias. See gulpfile.js#L7. Since you correctly pointed out that using complex icons in iconmoon app isn't a good idea I would prefer to stick to our existing solution for that.
regarding your question about integrating the workflow: I would prefer to run this as a github action so no "user-action" is required in order to execute the build. I would say integrating it directly into this repo isn't a problem and we could configure to execute the build action once a new pull request is merged into master.
build action
before but I will look into it. I've worked with TravisCI so I think I can make it work.I hope that answered your questions. Once you run the script, let me know what you think. Also, while Selenium is running please don't click away from the Firefox browser, sometimes this interrupts with the process.
Thanks for the feedback, I appreciate it.
@Thomas-Boi it's a pleasure working with you. I would prefer if we could find a solution without the need of having to "split" the devicon.json
into a new
and processed
list. In my mind we could use the devicon.json
as it is without the need of managing two lists of "new" and "done" icons.
What to you think of parsing the iconmoon.json
and compare it's icons with the icons listed in devicon.json
. The difference are our new
icons which we need to add to the iconmoon selection. This way we don't have to manually keep track of new
icons in a separated list (which could be confusing to new contributers).
Looking forward on your thoughts about this :)
@amacado
What to you think of parsing the iconmoon.json and compare it's icons with the icons listed in devicon.json. The difference are our new icons which we need to add to the iconmoon selection. This way we don't have to manually keep track of new icons in a separated list (which could be confusing to new contributers).
Unfortunately, I don't think this approach will work well in the long run. Let's say we go forward with this approach, here's what we'd need to do:
icomoon.json
and put it into an array with a length of n
.devicon.json
(also roughly length of n
) and for each icon, search the array above for whether it's in there (which is O(n)).This would be O(n^2) operation, which will take a long time as our repo grows.
How about we keep the devicon.json
's structure as it is but for the new fonts, they'd have an attribute for whether they have been made into an icon.
So it would look something like this:
[
{
notAdded: true,
// fonts details like name, tags, etc...
},
// current fonts
]
This would be O(n) whenever we want to search for the newly added fonts. Also, after we process the new fonts, we can remove the attribute notAdded
so only the new fonts would have it.
Another possible solution is to use a temporary database while we process the new svg files, something like redis or a local mongoDB. This would leave the devicon.json
untouched until the font making process is finished. The build script can just query the redis/mongoDB and get the details on the new fonts. However, this would require writing json data to command lines script, which is not ideal.
Also, will all users do this step whenever they add a new svgs or would a collaborator do it as a batch update? Depends on who do this job, we can make the steps before the build more complex (but more efficient) since we can write extra docs for it.
Let me know what you think of my suggestions :)
This would be O(n^2) operation, which will take a long time as our repo grows.
I think you underestimate the power of modern CPU's ;-) But even if it would take some time, this would not be something I worry about, since this is a build operation and not a productive task which is executed on usage. Anyway to get some numbers in I created a simple gulp task
as an example and time measurement.
var fs = require('fs');
function findNewIcons() {
var deviconJson = JSON.parse(fs.readFileSync('./devicon.json'))
var icomoonJson = JSON.parse(fs.readFileSync('./icomoon.json'))
var deviconArray = [];
deviconJson.forEach(function (obj, key) {
if (typeof obj.versions != null) {
if (typeof obj.versions.font != null) {
obj.versions.font.forEach(function(fontname) {
deviconArray.push(obj.name + '-' + fontname);
});
}
}
})
var icomoonArray = [];
icomoonJson.icons.forEach(function(obj, key) {
icomoonArray.push(obj.properties.name);
})
// filter icons which are in deviconArray but not in icomoonArray
var intersectArray = deviconArray.filter(value => !icomoonArray.includes(value));
console.log('In devicon font: ' + deviconArray.length);
console.log('In icomoon selection: ' + icomoonArray.length);
console.log('Intersection: ' + intersectArray.length);
console.log(intersectArray);
return Promise.resolve();
}
exports.findnew = findNewIcons;
Executing this task gulp findnew
took only around 7ms and here is the result:
[00:19:39] Starting 'findnew'...
In devicon font: 254
In icomoon selection: 239
Intersection: 29
[
'bitbucket-plain',
'bitbucket-plain-wordmark',
'c-plain-wordmark',
'c-line-wordmark',
'confluence-plain',
'confluence-plain-wordmark',
'cplusplus-plain-wordmark',
'cplusplus-line-wordmark',
'csharp-plain-wordmark',
'csharp-line-wordmark',
'django-plain-wordmark',
'django-line-wordmark',
'github-plain',
'github-plain-wordmark',
'heroku-original',
'heroku-original-wordmark',
'jasmine-plain-wordmark',
'jetbrains-plain-wordmark',
'jetbrains-line',
'jetbrains-line-wordmark',
'mocha-plain',
'nginx-original-wordmark',
'nginx-plain',
'nginx-plain-wordmark',
'sourcetree-plain',
'sourcetree-plain-wordmark',
'ssh-plain',
'ssh-plain-wordmark',
'twitter-plain'
]
[00:19:39] Finished 'findnew' after 6.54 ms
The intersection you can see in the output can be explained with our "alias" system, which might need a rework (we could simply put all the alias into the font aswell). But since this was just a prototype to show this would be possible you might want to work on it? Happy to hear your thoughts on it.
Sure, I can implement something similar to your gulp task in Python. I was worried about the run time as our repo grows but if we don't mind that, I can use your approach.
Now that we are using the devicon.json
as our main input file, I think we should discuss the types of the objects in it.
Currently, this is the format of a data object in the devicon.json
in TypeScript notation:
interface DeviconObject {
"name": string; // name of technology/brand/company
"tags": string[]; // informative tags about this font as an array
"versions": {
"svg": string[], // all the types that can be used as svgs
"font": string[]; // all the types that can be used as fonts
}
}
Here's the new format:
interface NewDeviconObject extends DeviconObject {
"color": string; // color for the colored version
"originalSameAsPlain": boolean; // whether we can treat the original version as the plain version aka create an alias
}
This means that we would have two different types of objects in the devicon.json
. Would this be ok for you? I can look into converting the current DeviconObject
into the NewDeviconObject
but it might take a while. I can look at the devicon-colors.css
and devicon-alias.css
to figure out how to construct the new one. However, from now on we must stick with the NewDeviconObject
format.
One last thing: I would like to confirm our stack before I add more functionalities to the build script. Here are what we are using:
devicon-colors.css
Let me know what you think and I can start working on the new features for the build script.
@Thomas-Boi if I got you right, then you want to replace devicon-colors.css
and devicon-alias.css
with an extended version of devicon.json
? Sounds like a great idea. But we need transform the existing data into the new schema. I don't want to have multiple concepts doing the same. When you keep the color
and alias
in the new version of devicon.json would you generate based on this the devicon-colors.css and devicon-alias.css and then combining them using the gulp task?
What to you think of a more flexible "NewDeviconObject" like this:
interface NewDeviconObject extends DeviconObject {
"color": string; // color for the colored version
"alias": [
{
"base": string;
"alias": string;
},
{
"base": string;
"alias": string;
}
]
}
So we could arrange a mapping between a "base" version and name an "alias" for it. For example
{
base: "original-wordmark",
alias: "plain-wordmark"
}
What do you think of this idea?
@amacado I'm not planning to replace the devicon-alias.css
and the devicon-colors.css
with the devicon.json
. The NewDeviconObject
would be used so that the build script can add new info to these css files instead of us doing so manually.
For example, our current process to make a color font is to:
devicon-colors.css
Instead of doing this manually, I plan to use the build script to append it to the file instead. The color attribute in the devicon.json
is used s the script knows which color to add to the devicon-colors.css
.
Same goes for the devicon-alias.css
. The process will parse the css returned by the Icomoon, find the base version, construct the alias version and append that to the devicon-alias.css
.
I like your version of the alias attribute; it's more explicit and makes more sense. As for the devicon-colors.css
and the devicon-alias.css
, we can't remove them completely since the current architecture relied on them. However, this is possible if we rework the architecture, as shown below.
Summary of the below points: we can't remove the css files unless we are willing to change the architecture. This would involve a website overhaul and a server == lots of work.
Detailed explanations: This is the current architecture:
For this architecture, the user has no choice over which fonts they want.
The final file devicon-min.css
contains the css for ALL of the fonts in the repo. The user would then download this css file through git clone or the CDN that Konpa set up. Since the devicon-min.css
depends on the devicon-colors.css
and devicon-alias.css
, we can't remove them from the process. We could delete them after every build process but that means we must build them from scratch every time as well.
Here's a possible architecture:
The user gets to choose which fonts that they want and we would only build the fonts and css files for the chosen ones only. The user can then download only the files that they needed from the website.
To create this architecture, we'd need to:
devicon.json
so that all objects follow the NewDeviconObject
interface.Since we are building on request, we only need a single source of truth: the devicon.json
. This means that we don't need to keep any css files in the repo since we are building from scratch. This new architecture would leave the current CDN link obsolete unless we continue to keep a big master list of the css and font files.
Final Words
In my opinion, I would prefer keeping the current architecture. We can consider a new architecture once we finish the build task.
@amacado I just reread your comment and I think I misunderstood it as removing the alias and colors css files completely.
When you keep the color and alias in the new version of devicon.json would you generate based on this the devicon-colors.css and devicon-alias.css and then combining them using the gulp task?
This is a good idea. I do have some concern about the runtime as our repo goes but I supposed it's manageable if we use async functionality of NodeJS.
But we need transform the existing data into the new schema
I think it's achievable using regexp. I have tried combining the devicon.json
and the devicon-colors.css
before. Regexp work well for this since there's a visible pattern.
However, the devicon-alias.css
would need some more work. I think we first parse the alias css, get the name and the content, then try and match that to the devicon.css.
Unfortunately, I don't have the script to parse the devicon-colors.css
anymore because I chose the way of appending colors instead. However, I can work on it once I get the build script finished. If you want to, you can try it as well.
@Thomas-Boi thanks again for your input and well written thoughts. I think you answered the question yourself ;-) Yes, I was thinking about regenerating the devicon-colors.css
and devicon-alias.css
based on the devicon.json
so we only need to maintain one single source of truth (exactly as you pointed out).
Looking forward to see your solution about the auto-transformation process, I currently have not enough time to develop one on my own but have deep trust in you finding a solution.
Hello all,
I would like to post an update about the devicon build script:
devicon.json
and the icomoon.json
base
version is uploaded when we are uploading to IcomoonMy next tasks are:
colors
and alias
css files then append it to the devicon.cssdevicon.json
such as logos without font versions
Hello all,
I have been working on a script that would automate the build process of the icomoon.json and the fonts folder. I would like to merge it into the devicon repo so that it makes it easier to add new fonts.
Summary
The script would automate the tasks of using the Icomoon website. It will upload the icomoon.json, upload any svgs specified in an input.json (more on that), download the zip and extract the needed files. All of this is achieved using Selenium and Python.
input.json
The input.json determines which svgs would be added to the project. Here is how it works currently:
Go into input.json and enter the new fonts into it:
Format:
Example:
What is the 'color' attribute
Currently, we have a
style.css
in order to have colored fonts. I plan to use the script to append new colored classes to the style.css. At the moment, this is still in the backlog because I wanted to explore whether we can add colors within the fonts itself. However, this seems like it would add a lot of complexity to the project (Icomoon said it'd create more complex fonts) so I think we might just keep using the css.I want to test it first
Here's the link to my repo.
Everything should be there to test the script. You'd need to install Python and Selenium though.
Integration
Now that the script is functional, I would like to ask you for advice on how to integrate it to the devicon repo. Here are my suggestions:
OR
Have a git branch specifically for building the icomoon.json. Uploaders can still upload but they won't have access to the build script. This creates 3 branches: a gh-pages branch for users to browse the icons, a master branch for user to upload icons, and a build branch for maintainer to build new icons.
I also have a NodeJS + Selenium version (not completed as the Python ver) which would do the same thing but I like the cleanliness of Python better.
What's next
If you guys accept the script, here are some areas that can be improved: -Add functionality to append colored classes to style.css -Make the browser runs on headless mode (so you don't have to sit and wait for the UI) or into a Docker image (which can runs in the background)
Feel free to leave any suggestions/comments