Skip to content

Conversation

@apoorvcodes
Copy link
Member

No description provided.

@netlify
Copy link

netlify bot commented Feb 26, 2022

✔️ Deploy Preview for gominima ready!

🔨 Explore the source changes: 0149ec9

🔍 Inspect the deploy log: https://app.netlify.com/sites/gominima/deploys/621b3b386803030008c3f47e

😎 Browse the preview: https://deploy-preview-10--gominima.netlify.app/

@savioxavier savioxavier changed the title feat: Site cleanup && footer && feature component feat: add footer and feature components and cleanup site Feb 26, 2022
@megatank58
Copy link
Contributor

Axios is one of the slowest performing http request libraries which doesn't make it a great candidate here. Undici is one of the fastest due to its many optimizations. In addition, it provides many spec compliant structures like FormData that are cross compatible with the web standards. (And it'll be built into node soon) So we should replace axios with undici here.

@savioxavier
Copy link
Member

Axios is one of the slowest performing http request libraries which doesn't make it a great candidate here. Undici is one of the fastest due to its many optimizations. In addition, it provides many spec compliant structures like FormData that are cross compatible with the web standards. (And it'll be built into node soon) So we should replace axios with undici here.

Isn't Axios one of most popular libraries out there, used by millions? At least it's better than node-fetch. Unidici's barely starting out. I think we should stick with Axios, I don't see why we should switch considering it works just fine.

@megatank58
Copy link
Contributor

Axios is one of the slowest performing http request libraries which doesn't make it a great candidate here. Undici is one of the fastest due to its many optimizations. In addition, it provides many spec compliant structures like FormData that are cross compatible with the web standards. (And it'll be built into node soon) So we should replace axios with undici here.

Isn't Axios one of most popular libraries out there, used by millions? At least it's better than node-fetch. Unidici's barely starting out. I think we should stick with Axios, I don't see why we should switch considering it works just fine.

Axios downloads: 25,416,438
node-fetch downloads: 35,972,655

Undici is the way to go as it is going to be the standard in node v18 and it'll be built-in, we also don't need any fancy features yet so it's just future proof. Also how's axios better than node-fetch 🤔

@apoorvcodes
Copy link
Member Author

I'll be switching it Undici, tbh that what the plan was but for quick testing I just used axios

@apoorvcodes apoorvcodes marked this pull request as ready for review February 27, 2022 07:11
@apoorvcodes
Copy link
Member Author

@megatank58 This is the rough new site there are some fixes needed like mobile stuff which will be done by @savioxavier I believe

@savioxavier
Copy link
Member

savioxavier commented Feb 27, 2022

@megatank58 This is the rough new site there are some fixes needed like mobile stuff which will be done by @savioxavier I believe

Needs a bit of code cleanup, component responsiveness, text and button scaling and wording fixes. Looks fine for me now, as I'll be fixing them in a later PR

@savioxavier
Copy link
Member

Squash and merge this btw, @megatank58. Too many commits.

@apoorvcodes
Copy link
Member Author

https://deploy-preview-10--gominima.netlify.app

Squash and merge this btw, @megatank58. Too many commits.

Haha ikr did a lot of fixes

@savioxavier
Copy link
Member

@apoorvcodes was axios replaced with undici, as per megatank's comment?

@apoorvcodes
Copy link
Member Author

@apoorvcodes was axios replaced with undici, as per megatank's comment?

I didn't mega will do it

@megatank58
Copy link
Contributor

Squash and merge this btw, @megatank58. Too many commits.

All of the coming merges on any of our repositories (with the exception of /docs) will be squashed

@savioxavier
Copy link
Member

Axios downloads: 25,416,438
node-fetch downloads: 35,972,655

Having more downloads doesn't necessarily mean it's more popular among developers. I just suggested Axios because I've used it over node-fetch in most of my projects. A lot of developers prefer Axios over built-in APIs for its ease of use.

Undici is the way to go as it is going to be the standard in node v18 and it'll be built-in, we also don't need any fancy features yet so it's just future proof.

I didn't see the point of switching to another library, since the only web request is a simple GitHub API fetch, which isn't too complex for axios to handle. However, if Undici seems like a better choice, if more API requests are to be added to the site in the future, and http optimization is key, then go for it!

Also how's axios better than node-fetch 🤔

I could cite a bunch of articles regarding this, but I don't think it's necessary.

We'll go with Undeci, since you both seem to agree with it. I just wanted to clarify my point here.

@megatank58
Copy link
Contributor

megatank58 commented Feb 27, 2022

Axios downloads: 25,416,438
node-fetch downloads: 35,972,655

Having more downloads doesn't necessarily mean it's more popular among developers. I just suggested Axios because I've used it over node-fetch in most of my projects. A lot of developers prefer Axios over built-in APIs for its ease of use.

I suggested node-fetch mainly because I'm more familiar with it and it's used in discord.js, while i do agree more downloads is not more popularity, that's a different topic.

Undici is the way to go as it is going to be the standard in node v18 and it'll be built-in, we also don't need any fancy features yet so it's just future proof.

I didn't see the point of switching to another library, since the only web request is a simple GitHub API fetch, which isn't too complex for axios to handle. However, if Undici seems like a better choice, if more API requests are to be added to the site in the future, and http optimization is key, then go for it!

The docs are rendered by fetching JSON which contains the documentation which will grow a lot in size as time goes own with multiple JSON for multiple projects and their own multiple branches and versions

Copy link
Member

@savioxavier savioxavier left a comment

Choose a reason for hiding this comment

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

I kinda like the new Dracula dark theme, nice work, Apoorv

Waiting for @megatank58 to review and merge.

Also, check if the PR title is appropriate for the merge, please.

@megatank58 megatank58 changed the title feat: add footer and feature components and cleanup site feat: add footer and feature components Feb 27, 2022
@megatank58 megatank58 merged commit a0697c1 into main Feb 27, 2022
@megatank58 megatank58 deleted the site-cleanup branch February 27, 2022 08:50
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

Successfully merging this pull request may close these issues.

4 participants