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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

First pass at README changes #6

Merged
merged 10 commits into from Nov 25, 2021
Merged

First pass at README changes #6

merged 10 commits into from Nov 25, 2021

Conversation

SeanNaren
Copy link
Contributor

@SeanNaren SeanNaren commented Nov 23, 2021

Before submitting

TODO:

  1. Let's get a logo for the repo like lightning: https://github.com/PyTorchLightning/pytorch-lightning/blob/master/docs/source/_static/images/logo.png
  2. Add the eco system badges somewhere
  3. Validate commands all work
  • Was this discussed/approved via a GitHub issue? (no need for typos and docs improvements)
  • Did you create/update your configuration file?
  • Did you add your config to CI - GitHub action and Azure pipeline?
  • Are all integration tests passing?

What does this PR do? [optional]

Fixes # (issue).

Did you have fun?

Make sure you had fun coding 馃檭

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@Borda Borda added the documentation Improvements or additions to documentation label Nov 23, 2021
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@tchaton
Copy link
Contributor

tchaton commented Nov 23, 2021

@SeanNaren any updates on the logo ?

@Borda
Copy link
Member

Borda commented Nov 23, 2021

@SeanNaren fyi, #10 is adding several discussed features 馃惏

SeanNaren and others added 6 commits November 24, 2021 17:42
Co-authored-by: thomas chaton <thomas@grid.ai>
Co-authored-by: thomas chaton <thomas@grid.ai>
Co-authored-by: thomas chaton <thomas@grid.ai>
Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>
Copy link
Member

@Borda Borda left a comment

Choose a reason for hiding this comment

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

LGTM after #6 (comment)

@SeanNaren
Copy link
Contributor Author

Should be good to go @Borda I'd add a minimal template.yaml with things commented out that are unnecessary for a normal repo.

@Borda Borda marked this pull request as ready for review November 25, 2021 00:29
@Borda Borda requested a review from tchaton November 25, 2021 00:29
Copy link
Member

@awaelchli awaelchli left a comment

Choose a reason for hiding this comment

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

Looks interesting!

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
- "MyUsername/my_project-master.yaml"
```

5. Add the responsible person(s) to [CODEOWNERS](.github/CODEOWNERS) for your organization folder or just the project.
Copy link
Member

Choose a reason for hiding this comment

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

We discovered that this won't do anything special unless the user is actually part of the repo (with write access).

Copy link
Member

Choose a reason for hiding this comment

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

true, but they are at least automatically added for the review..

Copy link
Member

Choose a reason for hiding this comment

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

No they are not, that's what I'm saying. See example here:
PR Lightning-AI/pytorch-lightning#10369
Codeowners: https://github.com/PyTorchLightning/pytorch-lightning/blob/85d7c4dce4661ef5c12fcbd0d9e164ffc371e1af/.github/CODEOWNERS#L28-L30

From
https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners:

The people you choose as code owners must have write permissions for the repository. When the code owner is a team, that team must be visible and it must have write permissions, even if all the individual members of the team already have write permissions directly, through organization membership, or through another team membership.

Copy link
Member

Choose a reason for hiding this comment

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

I see, co we would need to come with another solution... :/

README.md Show resolved Hide resolved
Co-authored-by: Adrian W盲lchli <aedu.waelchli@gmail.com>
@Borda Borda merged commit b37a4ce into main Nov 25, 2021
@Borda Borda deleted the refactor/readme branch November 25, 2021 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants