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

Created new sub category in resources #2132

Closed
wants to merge 4 commits into from

Conversation

Amanmawar17
Copy link

@Amanmawar17 Amanmawar17 commented Oct 27, 2023

Fixes Issue

closes #2107

Changes proposed

Added a new sub-category in the resources tab which was self-paced courses.

I made a new JSON file in the resources folder named self-paced courses.
I committed changes data.ts and index.ts to import new self-paced-course.json for this to be shown in the category.

Screenshots

Screenshot 2023-10-27 125930

Note to reviewers

Suggest any required changes!!
Thank You.

@vercel
Copy link

vercel bot commented Oct 27, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
linkshub ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 31, 2023 9:15am

@github-actions github-actions bot added chore Might take time to finish goal: new-category Addition of new categories related contributions priority: low Addition of new links/categories or doing any small task (e.g fixing typos) labels Oct 27, 2023
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thank you, Amanmawar17, for creating this pull request and contributing to LinksHub! 💗

The maintainers will review this Pull Request and provide feedback as soon as possible! 😇
We appreciate your patience and contribution, Keep up the great work! 😀

@Anmol-Baranwal
Copy link
Collaborator

@Amanmawar17
Following the PR template is a must. It includes special words like 'fixes' or 'resolves' that connect your issue to the PR, as explained in the official docs.

If you don't link the issue correctly, even after the PR is merged, the issue might stay open, causing huge problems.

Including it in title won't work.

You can see here, if the issue is linked.
image

I've done it for you.

Just a heads-up to help you link issue properly next time. Have a great day!

Copy link
Collaborator

@Anmol-Baranwal Anmol-Baranwal left a comment

Choose a reason for hiding this comment

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

@Amanmawar17
Please add the description of the subcategory.

It will be in TopBar/CategoryDescriptions.ts

@Amanmawar17
Copy link
Author

@Amanmawar17 Please add the description of the subcategory.

It will be in TopBar/CategoryDescriptions.ts

I resolved this issue but can you tell me how I made a PR for new changes?

@Anmol-Baranwal
Copy link
Collaborator

Anmol-Baranwal commented Oct 27, 2023

@Amanmawar17 Please add the description of the subcategory.
It will be in TopBar/CategoryDescriptions.ts

I resolved this issue but can you tell me how I made a PR for new changes?

You need to make changes in the same branch. When you push it to github in your fork, it will automatically be included in this PR.

Copy link
Collaborator

@CBID2 CBID2 left a comment

Choose a reason for hiding this comment

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

Hi @Amanmawar17. The descriptions in your PR needs more improvement, so I provided suggestions to help you. Other than that, you're off to a great start! 😊

database/resources/self-paced-courses.json Outdated Show resolved Hide resolved
database/resources/self-paced-courses.json Outdated Show resolved Hide resolved
database/resources/self-paced-courses.json Outdated Show resolved Hide resolved
Copy link
Collaborator

@Anmol-Baranwal Anmol-Baranwal left a comment

Choose a reason for hiding this comment

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

It's advisable to use "e-book" or "ebook" to properly convey the meaning of an electronic book.

What do you think? e book, e-book, ebook

image

image

I am confused about the extra change. The description is definitely fixed, but which one is more suitable?

@Amanmawar17
Copy link
Author

if we choose E-book then the description would not be there, and also E-Book is also not there, showing this
Screenshot 2023-10-29 112542

so what revert back this E-book change and then open another issue for it?

Anmol-Baranwal
Anmol-Baranwal previously approved these changes Oct 29, 2023
Copy link
Collaborator

@Anmol-Baranwal Anmol-Baranwal left a comment

Choose a reason for hiding this comment

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

🚀 Looks good to me.

@Amanmawar17
It's fine, we will change it if it feels odd later on.

@Anmol-Baranwal
Copy link
Collaborator

@CBID2
You're up for review!

CBID2
CBID2 previously approved these changes Oct 30, 2023
Copy link
Collaborator

@CBID2 CBID2 left a comment

Choose a reason for hiding this comment

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

Looks great

@Anmol-Baranwal Anmol-Baranwal added the status: ready-to-merge Approved & its ready-to-merge label Oct 30, 2023
@CBID2
Copy link
Collaborator

CBID2 commented Oct 30, 2023

Found some conflicts that need merging @Amanmawar17

@Anmol-Baranwal
Copy link
Collaborator

@Amanmawar17
You can see the changes, and push it accordingly if you want, it will fix merge conflicts in case you have trouble using command line for the same.

@Amanmawar17 Amanmawar17 dismissed stale reviews from CBID2 and Anmol-Baranwal via aefc42a October 31, 2023 09:15
Copy link
Owner

@rupali-codes rupali-codes left a comment

Choose a reason for hiding this comment

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

lgtm

@rupali-codes
Copy link
Owner

HI @Amanmawar17 please resolve the merge conflicts

rupali-codes

This comment was marked as duplicate.

Copy link
Owner

@rupali-codes rupali-codes left a comment

Choose a reason for hiding this comment

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

lgtm

@rupali-codes
Copy link
Owner

@Amanmawar17 please resolve merge conflicts

@Anmol-Baranwal
Copy link
Collaborator

I'm closing this issue due to inactivity. Please feel free to reopen it if you'd like to contribute or have further discussions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Might take time to finish goal: new-category Addition of new categories related contributions priority: low Addition of new links/categories or doing any small task (e.g fixing typos) status: ready-to-merge Approved & its ready-to-merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Self Paced Courses], New Sub category in Resources
4 participants