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

Added document section with required Links #143

Closed
wants to merge 8 commits into from
Closed

Added document section with required Links #143

wants to merge 8 commits into from

Conversation

StarTrooper08
Copy link

@StarTrooper08 StarTrooper08 commented May 20, 2021

Description

Added document section with required Links

Fixes #71

Type of Change:

  • Documentation

Code/Quality Assurance Only

  • Bug fix (non-breaking change which fixes an issue)
  • This change requires a documentation update (software upgrade on readme file)
  • New feature (non-breaking change which adds functionality pre-approved by mentors)

How Has This Been Tested?

N/A

Checklist:

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials
  • I have commented my code or provided relevant documentation, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • Any dependent changes have been merged
  • I have written Kotlin Docs whenever is applicable

Links need to be added
links of the respective points added
Added the links
@StarTrooper08
Copy link
Author

This PR should solve issue no. #71

@Aaishpra
Copy link
Member

Can you add the issue number in the PR description after Fixes # so that the PR closes the issue

@StarTrooper08
Copy link
Author

Ok I will do it as soon as possible

@StarTrooper08 StarTrooper08 changed the title Added document section with required Links Added document section with required Links #71 May 20, 2021
@StarTrooper08 StarTrooper08 changed the title Added document section with required Links #71 Added document section with required Links (Fixes #71) May 20, 2021
@StarTrooper08
Copy link
Author

Done

@codesankalp
Copy link
Member

@StarTrooper08 Add Fixes #<issue_number> in PR description and not in the PR title.
If you read the description you will see Fixes just add the issue number there.

@StarTrooper08
Copy link
Author

Done ✅

README.md Outdated
@@ -17,6 +17,14 @@ Anitab-Forms is an application that simplifies the processing and selection proc

Documentation for the project is hosted [here](https://osp-web-docs.surge.sh/). We use `Docusaurus` for maintaining the documentation of the project.

### Few Useful Links:
1. **`anitab-org wiki`** link - https://github.com/anitab-org/anitab-forms-web/wiki
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
1. **`anitab-org wiki`** link - https://github.com/anitab-org/anitab-forms-web/wiki
1. **anitab-org wiki** [link](https://github.com/anitab-org/anitab-forms-web/wiki)

Do the same with other links also

Copy link
Author

Choose a reason for hiding this comment

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

Ok

@StarTrooper08
Copy link
Author

@Aaishpra do the changes I have made now is right??

README.md Outdated
5. **`Question in Zulip chats`** link - https://anitab-org.zulipchat.com/#narrow/stream/223070-questions
6. **`Announcement in Zulip chats`** link - https://anitab-org.zulipchat.com/#narrow/stream/213491-announcements
1. **anitab-org wiki** : [Visit](https://github.com/anitab-org/anitab-forms-web/wiki)
2. **Open Sessions** : [Visit](https://meet.google.com/eqb-nuut-kqm)
Copy link
Member

Choose a reason for hiding this comment

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

@Aaishpra Is an open session link necessary in this? Because this may change in future.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it can definitely change, Shall we add the document link of our open sessions instead of this?

README.md Outdated
3. **GSoC** youtube video link : [Visit](https://youtu.be/3A746GppZ0Y)
4. **Setting up the project** youtube video link : [Visit](https://youtu.be/_b2RQGbYN9w)
5. **Question in Zulip chats** link : [Visit](https://anitab-org.zulipchat.com/#narrow/stream/223070-questions)
6. **Announcement in Zulip chats** link : [Visit](https://anitab-org.zulipchat.com/#narrow/stream/213491-announcements)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
6. **Announcement in Zulip chats** link : [Visit](https://anitab-org.zulipchat.com/#narrow/stream/213491-announcements)
6. [Announcement in Zulip chats](https://anitab-org.zulipchat.com/#narrow/stream/213491-announcements): In this stream, all the important decision are announced by admins related to anitab-org opensource.

What do you think @Aaishpra about this representation i.e. link followed by some detail? This can be done for all the links.

Copy link
Author

Choose a reason for hiding this comment

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

Can I do this changes?

Copy link
Member

@Aaishpra Aaishpra May 24, 2021

Choose a reason for hiding this comment

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

This is a good idea but I am still not sure about adding announcement and questions stream links in the readme as they are common streams and not specifically dedicated to this project. Just adding links to our project stream should be fine. 🤔 What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Although we can definitely add details for other streams.
@StarTrooper08 can you add these details by yourself or would you like some help?

Copy link
Author

Choose a reason for hiding this comment

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

This is a good idea but I am still not sure about adding announcement and questions stream links in the readme as they are common streams and not specifically dedicated to this project. Just adding links to our project stream should be fine. 🤔 What do you think?

I have one idea ,
we can just add one zulip chat link instead of those 2 channels links of zulip chat.

What do you say??

Copy link
Member

Choose a reason for hiding this comment

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

Ya, and the link should take you to the anitab-forms stream.(although I think we already have one in the badge).
Let's see what @codesankalp says

Copy link
Author

Choose a reason for hiding this comment

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

Although we can definitely add details for other streams.
@StarTrooper08 can you add these details by yourself or would you like some help?

Ok I will first try of my own and if stuck somewhere I will take help .

Copy link
Author

Choose a reason for hiding this comment

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

Ya, and the link should take you to the anitab-forms stream.(although I think we already have one in the badge).
Let's see what @codesankalp says

Ok

codesankalp
codesankalp previously approved these changes May 26, 2021
@codesankalp codesankalp added the Status: Needs Review PR needs an additional review or a maintainer's review. label May 26, 2021
README.md Outdated
@@ -17,6 +17,14 @@ Anitab-Forms is an application that simplifies the processing and selection proc

Documentation for the project is hosted [here](https://osp-web-docs.surge.sh/). We use `Docusaurus` for maintaining the documentation of the project.

### Few Useful Links:
1. [anitab-org wiki](https://github.com/anitab-org/anitab-forms-web/wiki) : Wiki page
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
1. [anitab-org wiki](https://github.com/anitab-org/anitab-forms-web/wiki) : Wiki page
1. [anitab-forms wiki](https://github.com/anitab-org/anitab-forms-web/wiki) : Wiki page

@vj-codes vj-codes added Status: Changes Requested Changes are required to be done by the PR author. and removed Status: Needs Review PR needs an additional review or a maintainer's review. labels Aug 14, 2021
@Aaishpra
Copy link
Member

@StarTrooper08 any updates?

[anitab-org wiki] --> [anitab-forms wiki]
@StarTrooper08
Copy link
Author

I m sorry, for the delay.

Aaishpra
Aaishpra previously approved these changes Sep 16, 2021
Copy link
Member

@Aaishpra Aaishpra left a comment

Choose a reason for hiding this comment

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

Great work

@Aaishpra
Copy link
Member

@codesankalp @isabelcosta have a look here

README.md Outdated Show resolved Hide resolved
isabelcosta
isabelcosta previously approved these changes Sep 19, 2021
Copy link
Member

@isabelcosta isabelcosta 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 for contributing @StarTrooper08 🙌🏾

@isabelcosta isabelcosta changed the title Added document section with required Links (Fixes #71) Added document section with required Links Sep 19, 2021
@isabelcosta
Copy link
Member

@StarTrooper08 Can you run prettier, so the lint check passes please?

@StarTrooper08
Copy link
Author

StarTrooper08 commented Sep 19, 2021

Ok @isabelcosta I'll do it.
But how can I do it 😅.
Can I get help!

@isabelcosta
Copy link
Member

@StarTrooper08 of course you can get help :) I just have to ping other maintainers, because I am not very well versed with frontend 😅 @Aaishpra @codesankalp
I took a look at the readme and it does say you can run: npm run lint:fix. Can you do this and push the changes?

Copy link
Member

@Aaishpra Aaishpra left a comment

Choose a reason for hiding this comment

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

@StarTrooper08 Isabel told you the correct command or if you use yarn use yarn lint-fix once the tests are passing I can approve the PR,

@codesankalp
Copy link
Member

@StarTrooper08 Rebasing your branch with the current developed branch will also make the lining test pass.

@StarTrooper08
Copy link
Author

Ok

@isabelcosta
Copy link
Member

@StarTrooper08 are you still working on this? If not, let us know so we can make the ticket available again :)

@StarTrooper08
Copy link
Author

Yes yes I was stucked in some other work.

@StarTrooper08
Copy link
Author

I used the command npm run lint:fix.
But still, it's showing 1 failing check.

@codesankalp
Copy link
Member

I used the command npm run lint:fix. But still, it's showing 1 failing check.

Have you rebased the branch?
After rebasing try to run the command and then push the rebased branch.

@StarTrooper08
Copy link
Author

@isabelcosta @codesankalp please have a look

@codesankalp
Copy link
Member

@StarTrooper08 Can you please resolve the merge conflicts?

@codesankalp
Copy link
Member

codesankalp commented Oct 10, 2021

@StarTrooper08 Can you please resolve the merge conflicts?

To do this replace the develop branch package-lock.json file with your local file.
Also, I see you haven't rebased the branch (try this):

git remote add upstream [email protected]:anitab-org/anitab-forms-web.git 
git fetch upstream
git rebase upstream/develop
npm install
npm run lint:fix
git push

@StarTrooper08 StarTrooper08 closed this by deleting the head repository May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Changes Requested Changes are required to be done by the PR author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docs: Add a documentation Section in the README.md
5 participants