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

Fix:Addessed issues in README #1063

Merged
merged 4 commits into from
Aug 22, 2024

Conversation

LanieOkorodudu
Copy link
Collaborator

I started to address these README issues;
Twitter - change to X, move to after link for slack
Website - should include link to https://universalviewer.io/

Copy link

vercel bot commented Jul 23, 2024

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

Name Status Preview Comments Updated (UTC)
universalviewer ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 24, 2024 8:22am

@LanieOkorodudu
Copy link
Collaborator Author

There are still issues need to address in README.md and these are;

  1. Broken links require fixing
  2. Getting started - requires review; more context to be useful; Distinguish between info required to get started, to contribute etc.
  3. 'Docs' - confusing, should go directly to typescript stuff; perhaps change link to 'typescript doc' or ' generated code doc'

@demiankatz
Copy link
Contributor

Thanks for getting this started, @LanieOkorodudu!

A couple suggested next steps:

1.) Change the URL https://github.com/UniversalViewer/universalviewer/blob/master/LICENSE.txt to https://github.com/UniversalViewer/universalviewer/blob/main/LICENSE.txt

2.) Under "Getting Started" I would change the docs link to https://docs.universalviewer.io/modules.html and maybe rephrase the whole sentence to something like "Read the technical docs to learn more about the code and available configuration options."

A bigger issue to discuss/think about: the Contributing section references (and links to) a number of issue labels, but none of these labels are currently being used. Should we either make an effort to start labeling issues, or should we remove/reduce this section of the README? Right now it just contributes to a feeling that parts of the project have been abandoned. Maybe for now the best thing to do is remove all the sub-bullets, and link "issue" in "Find an issue to work on and submit a pull request" to https://github.com/UniversalViewer/universalviewer/issues.

@LanieOkorodudu
Copy link
Collaborator Author

Thanks @demiankatz for your suggestions. Regarding the Contributing section, I agree that the current references to issue labels can be misleading since they are not being used. As you mentioned, I think this should be discussed and agreed upon in the next SG meeting, whether we should remove or reduce this section of the README. Do you recommend that I update the link as you suggested now, or should I wait until the Contributing section is finalized?

@demiankatz
Copy link
Contributor

Thanks @demiankatz for your suggestions. Regarding the Contributing section, I agree that the current references to issue labels can be misleading since they are not being used. As you mentioned, I think this should be discussed and agreed upon in the next SG meeting, whether we should remove or reduce this section of the README. Do you recommend that I update the link as you suggested now, or should I wait until the Contributing section is finalized?

I think it's likely that we'll agree to the changes, so I don't think there's any harm in making the edits now. Then we can discuss and, if we agree, merge during the next meeting. (And if we don't agree, it's not hard to undo controversial changes).

Thanks again!

@LanieOkorodudu
Copy link
Collaborator Author

Thanks @demiankatz for your suggestions. Regarding the Contributing section, I agree that the current references to issue labels can be misleading since they are not being used. As you mentioned, I think this should be discussed and agreed upon in the next SG meeting, whether we should remove or reduce this section of the README. Do you recommend that I update the link as you suggested now, or should I wait until the Contributing section is finalized?

I think it's likely that we'll agree to the changes, so I don't think there's any harm in making the edits now. Then we can discuss and, if we agree, merge during the next meeting. (And if we don't agree, it's not hard to undo controversial changes).

Thanks again!

Thanks @demiankatz for your suggestions. Regarding the Contributing section, I agree that the current references to issue labels can be misleading since they are not being used. As you mentioned, I think this should be discussed and agreed upon in the next SG meeting, whether we should remove or reduce this section of the README. Do you recommend that I update the link as you suggested now, or should I wait until the Contributing section is finalized?

I think it's likely that we'll agree to the changes, so I don't think there's any harm in making the edits now. Then we can discuss and, if we agree, merge during the next meeting. (And if we don't agree, it's not hard to undo controversial changes).

Thanks again!

Do you think this link should be remove since they are not being used. First time contributing to the UV? Pick a beginner friendly issue to get you familiar with codebase and our contributing process.
Want to become a Committer? Solve an issue showing that you understand UV objectives and architecture. Here is a good list to start.

@demiankatz
Copy link
Contributor

Do you think this link should be remove since they are not being used. First time contributing to the UV? Pick a beginner friendly issue to get you familiar with codebase and our contributing process. Want to become a Committer? Solve an issue showing that you understand UV objectives and architecture. Here is a good list to start.

Yes, that would be my suggestion. Keep the top-level bullet, with an added link to the issues page, and then just delete all the sub-bullets below it that link to specific labels.

1.) Change the URL https://github.com/UniversalViewer/universalviewer/blob/master/LICENSE.txt to https://github.com/UniversalViewer/universalviewer/blob/main/LICENSE.txt
2.) Under "Getting Started" I would change the docs link to https://docs.universalviewer.io/modules.html and maybe rephrase the whole sentence to something like "Read the technical docs to learn more about the code and available 
3.) Added link to "Find an issue to work on and submit a pull request" to https://github.com/UniversalViewer/universalviewer/issues.
4.) deleted all the sub-bullets below it that link to specific labels
@LanieOkorodudu
Copy link
Collaborator Author

Do you think this link should be remove since they are not being used. First time contributing to the UV? Pick a beginner friendly issue to get you familiar with codebase and our contributing process. Want to become a Committer? Solve an issue showing that you understand UV objectives and architecture. Here is a good list to start.

Yes, that would be my suggestion. Keep the top-level bullet, with an added link to the issues page, and then just delete all the sub-bullets below it that link to specific labels.

I have made the changes as you suggested. The top-level bullet now includes a link to the issues page, and all the sub-bullets that linked to specific labels have been removed. Could you please review the changes to confirm if they are what you had in mind?

@demiankatz
Copy link
Contributor

Thanks, @LanieOkorodudu! I would perhaps change:

Find an issue to work on and submit a pull request to [issues page](https://github.com/UniversalViewer/universalviewer/issues)

to:

Find an [issue](https://github.com/UniversalViewer/universalviewer/issues) to work on and submit a pull request

@LanieOkorodudu
Copy link
Collaborator Author

Thanks, @LanieOkorodudu! I would perhaps change:

Find an issue to work on and submit a pull request to [issues page](https://github.com/UniversalViewer/universalviewer/issues)

to:

Find an [issue](https://github.com/UniversalViewer/universalviewer/issues) to work on and submit a pull request

Okay this make sense 💯 agree with you. Thanks again @demiankatz, I really appreciate your help 😃

@demiankatz
Copy link
Contributor

Looks good to me now! Thanks again, @LanieOkorodudu.

@LanieOkorodudu LanieOkorodudu marked this pull request as ready for review August 22, 2024 09:56
@LanieOkorodudu LanieOkorodudu merged commit e52fe6b into UniversalViewer:main Aug 22, 2024
2 checks passed
@demiankatz
Copy link
Contributor

Thanks again, @LanieOkorodudu! I just noticed that this PR was targeted to main instead of dev; we should try to get in the habit of merging things to dev first. Fortunately, no harm done here -- I just merged main backwards to dev to get things back in sync again. :-)

@LanieOkorodudu
Copy link
Collaborator Author

Thanks again, @LanieOkorodudu! I just noticed that this PR was targeted to main instead of dev; we should try to get in the habit of merging things to dev first. Fortunately, no harm done here -- I just merged main backwards to dev to get things back in sync again. :-)

oh my bad. I should have ask you first before merging. Thanks for letting me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

2 participants