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

Update ionicons #353

Closed
quozl opened this issue Jan 16, 2022 · 12 comments
Closed

Update ionicons #353

quozl opened this issue Jan 16, 2022 · 12 comments

Comments

@quozl
Copy link
Contributor

quozl commented Jan 16, 2022

ionicons.min.css is version 2.0.0, but latest release of ionicons is version 6.0.0

@shubhayu09
Copy link

Hey @quozl,
I would like to take up this issue. Can you pls elaborate on this issue in detail and tell me where to start as I'm new to this community.

@quozl
Copy link
Contributor Author

quozl commented Jan 19, 2022

Thanks. I don't mind who fixes this issue. The reason I created the issue was that I saw the difference in version numbers and I didn't have the knowledge and experience to know what to do about it, nor did I have the time to acquire them. So please be specific on how you want me to elaborate on the issue given that I don't have knowledge, experience, or the time to acquire them. If I did, I would have fixed it myself, as it would be faster than explaining it to someone who doesn't have the knowledge or experience. If you have the knowledge and experience, please proceed. As should anyone else. However, if you don't have the knowledge and experience, but you have the time to acquire these, then do so. Don't expect me to be able to verify in detail. All I did was notice the difference. In general, using old versions of software without updating them is dangerous because it can lead to software security vulnerabilities.

@shubhayu09
Copy link

Ok, I got it.
Will try my best to fix this.

@chimosky
Copy link
Member

Hi @shubhayu09 Welcome!

In addition to what quozl has said, you should also look at our contributing guide.

@quozl quozl mentioned this issue Jan 20, 2022
@quozl
Copy link
Contributor Author

quozl commented Jan 20, 2022

@shubhayu09, please review #354

@shubhayu09
Copy link

@shubhayu09, please review #354

Yeah sure

@nikkuAg
Copy link
Contributor

nikkuAg commented Jan 26, 2022

Hey @quozl,
Can I work on this issue?
I will also incorporate the changes for the icons in footer and side bar.

@quozl
Copy link
Contributor Author

quozl commented Jan 26, 2022

I'm tired of being asked. It's about time you read our guide to Contributing in particular where it starts out saying

"Please note there is no need to ask permission to work on an issue. You should check for pull requests linked to an issue you are addressing; if there are none, then assume nobody has done anything. Begin to fix the problem, test, make your commits, push your commits, then make a pull request. Mention an issue number in the pull request, but not the commit message. These practices allow the competition of ideas (Sugar Labs is a meritocracy)."

If your purpose is to block other contributors, such as @shubhayu09 here, that won't work. We'll take whichever pull request is best, or even combine pull requests. Let's get this done.

@nikkuAg
Copy link
Contributor

nikkuAg commented Feb 2, 2022

Hey @quozl @chimosky,

I want to confirm that is it required to use files like ionicons.min.css as there is an easier way by using a script and then importing the icons.

@chimosky
Copy link
Member

chimosky commented Feb 2, 2022

Currently the idea is to update the existing ionicons.min.css version and that includes using the file, if you think there's a better way then you can open a PR and add why you think that's better to your commit message as it'll be easier for us to test and review and move the conversation forward.

@quozl
Copy link
Contributor Author

quozl commented Feb 3, 2022

Sorry. I don't understand the question. I agree with @chimosky that it would be clearer if we saw a pull request, which you can mark as draft if it is not intended for merging. The issue here (#353) is that an old version is being used. Removing it would fix it, as would updating it. If removing it is proposed, I'd like to see reasons.

@nikkuAg
Copy link
Contributor

nikkuAg commented Feb 4, 2022

Hey @quozl @chimosky,

I have updated the changes for this issue in PR #354.
Please review it.

@chimosky chimosky closed this as completed Feb 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants