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

[REG-1635] Add SDK link to homepage #63

Merged
merged 1 commit into from
Mar 12, 2024
Merged

[REG-1635] Add SDK link to homepage #63

merged 1 commit into from
Mar 12, 2024

Conversation

vontell
Copy link
Contributor

@vontell vontell commented Mar 12, 2024

Adds the SDK url to the homepage for power users who want to jump right in. Assumes they know how to add git packages to Unity through the package manager. 

Screenshot 2024-03-12 at 12 35 25 PM

Find the pull request instructions here

Every reviewer and the owner of the PR should consider these points in their request (feel free to copy this checklist so you can fill it out yourself in the overall PR comment)

  • The code is extensible and backward compatible
  • New public interfaces are extensible and open to backward compatibility in the future
  • If preparing to remove a field in the future (i.e. this PR removes an argument), the argument stays but is no longer functional, and attaches a deprecation warning. A linear task is also created to track this deletion task.
  • Non-critical or potentially modifiable arguments are optional
  • Breaking changes and the approach to handling them have been verified with the team (in the Linear task, design doc, or PR itself)
  • The code is easy to read
  • Unit tests are added for expected and edge cases
  • Integration tests are added for expected and edge cases
  • Functions and classes are documented
  • Migrations for both up and down operations are completed
  • A documentation PR is created and being reviewed for anything in this PR that requires knowledge to use
  • Implications on other dependent code (i.e. sample games and sample bots) is considered, mentioned, and properly handled
  • Style changes and other non-blocking changes are marked as non-blocking from reviewers

Copy link

@rcornwall-reg rcornwall-reg left a comment

Choose a reason for hiding this comment

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

[Non Blocking]
Linking directly to the package instead of the root project is a good idea, but I feel like we should have another README inside the package folder as well. It's always a bit jarring to me when I hit a repo and there's no README

Copy link
Contributor

@nAmKcAz nAmKcAz left a comment

Choose a reason for hiding this comment

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

Approving, but the number of users that would do this without first reading what the package does seems like it would be small or near zero.

@vontell
Copy link
Contributor Author

vontell commented Mar 12, 2024

For reviewers: @rcornwall-reg the repo does have a readme, although it's in the root folder, not in the package subfolder.

@RG-nAmKcAz definitely see what you are saying - I have found that I do use this option in other packages though when I've used them before - when I know what I want, I've used it before, and I don't want to dig through the docs for the package link.

@rcornwall-reg
Copy link

rcornwall-reg commented Mar 12, 2024

@vontell I was specifically talking about adding a second README to the subfolder you're directly linking to, at least with install instructions

@vontell vontell merged commit 7a4331a into main Mar 12, 2024
1 check passed
@vontell vontell deleted the aaron/reg-1635 branch March 12, 2024 19:45
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.

3 participants