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.

Sign up for GitHub

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

Jump to bottom

Usage: add GitLab, Bitbucket sections #122

Merged
merged 8 commits into from
Oct 26, 2021
Merged

Usage: add GitLab, Bitbucket sections #122

merged 8 commits into from
Oct 26, 2021

Conversation

casperdcl
Copy link
Contributor

@casperdcl casperdcl commented Oct 11, 2021 •
edited
Loading

@casperdcl casperdcl self-assigned this Oct 11, 2021
@shcheklein shcheklein temporarily deployed to cml-dev-usage-5cihfrpcygybdobc October 11, 2021 15:18 Inactive
@casperdcl casperdcl added blocked Dependent on something else documentation Markdown files labels Oct 11, 2021
@casperdcl casperdcl mentioned this pull request Oct 11, 2021
Open
6 tasks
@casperdcl casperdcl added the p1-important High priority label Oct 11, 2021
@shcheklein shcheklein temporarily deployed to cml-dev-usage-5cihfrpcygybdobc October 12, 2021 17:21 Inactive
@casperdcl casperdcl mentioned this pull request Oct 12, 2021
Merged
@shcheklein shcheklein temporarily deployed to cml-dev-usage-5cihfrpcygybdobc October 12, 2021 17:24 Inactive
@casperdcl casperdcl marked this pull request as ready for review October 12, 2021 17:25
0x2b3bfa0
0x2b3bfa0 reviewed Oct 12, 2021
View reviewed changes
content/docs/usage.md Outdated Show resolved Hide resolved
0x2b3bfa0
0x2b3bfa0 requested changes Oct 12, 2021
View reviewed changes
Copy link
Member

@0x2b3bfa0 0x2b3bfa0 left a comment

Choose a reason for hiding this comment

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

🚨 I have seen a few occurrences of repo_token (lowercase) in the wild.

casperdcl
casperdcl commented Oct 12, 2021
View reviewed changes
Copy link
Contributor Author

@casperdcl casperdcl left a comment

Choose a reason for hiding this comment

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

GH review clickops

content/docs/usage.md Outdated Show resolved Hide resolved
content/docs/usage.md Outdated Show resolved Hide resolved
@shcheklein shcheklein temporarily deployed to cml-dev-usage-5cihfrpcygybdobc October 12, 2021 19:15 Inactive
@casperdcl
Copy link
Contributor Author

rotating_light I have seen a few occurrences of repo_token (lowercase) in the wild.

yes but I believe in all cases they are GL/BB variables so do not appear in any code.

We really should change everywhere to CML_TOKEN (more explicit & avoids namespace clashes)

@casperdcl casperdcl mentioned this pull request Oct 12, 2021
Closed
@0x2b3bfa0
Copy link
Member

yes but I believe in all cases they are GL/BB variables so do not appear in any code.

What do you mean by GitLab / Bitbucket variables?

We really should change everywhere to CML_TOKEN (more explicit & avoids namespace clashes)

Yes, probably. Linking to iterative/cml#763...

@casperdcl
Copy link
Contributor Author

casperdcl commented Oct 12, 2021 •
edited
Loading

What do you mean by GitLab / Bitbucket variables?

afaik GH uses "secrets" which need explicit mention in workflow files, while GL & BB use auto-exposed "variables." https://cml.dev/doc/self-hosted-runners#personal-access-token

@0x2b3bfa0

This comment has been minimized.

Sign in to view
0x2b3bfa0
0x2b3bfa0 approved these changes Oct 12, 2021
View reviewed changes
Copy link
Member

@0x2b3bfa0 0x2b3bfa0 left a comment

Choose a reason for hiding this comment

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

I'm not going to block this pull request because of mentions to repo_token in lowercase, but it would be nice to change them instead of introducing more.

@casperdcl
Copy link
Contributor Author

@0x2b3bfa0 I'm on the fence given iterative/cml#763 but happy for you to push the git sed commit :)

DavidGOrtega
DavidGOrtega reviewed Oct 15, 2021
View reviewed changes
Copy link
Contributor

@DavidGOrtega DavidGOrtega left a comment

Choose a reason for hiding this comment

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

small nitpicks

content/docs/usage.md
We helpfully provide CML and other useful libraries pre-installed on our
[custom Docker images](/doc/self-hosted-runners#docker-images). In the above
example, uncommenting the
`container: docker://ghcr.io/iterative/cml:0-dvc2-base1` field will make the
runner pull the CML Docker image. The image already has Node.js, Python 3, DVC
and CML set up on an Ubuntu LTS base for convenience.

### Example projects

- [Basic CML project](https://github.com/iterative/cml_base_case)
Copy link
Contributor

Choose a reason for hiding this comment

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

We should put the fashion case. It's a REAL case 😉
Lets open a ticket to put that repo under iterative. I would probably remove all of the rest after that.

content/docs/usage.md Outdated Show resolved Hide resolved
content/docs/usage.md Outdated Show resolved Hide resolved
@shcheklein shcheklein temporarily deployed to cml-dev-usage-5cihfrpcygybdobc October 16, 2021 13:28 Inactive
@shcheklein shcheklein temporarily deployed to cml-dev-usage-5cihfrpcygybdobc October 26, 2021 13:13 Inactive
@shcheklein shcheklein temporarily deployed to cml-dev-usage-5cihfrpcygybdobc October 26, 2021 13:19 Inactive
@casperdcl casperdcl merged commit eadedaf into master Oct 26, 2021
@casperdcl casperdcl deleted the usage branch October 26, 2021 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DavidGOrtega DavidGOrtega DavidGOrtega left review comments

@0x2b3bfa0 0x2b3bfa0 0x2b3bfa0 approved these changes

Assignees

@casperdcl casperdcl

Labels
blocked Dependent on something else documentation Markdown files p1-important High priority
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

use tabs
4 participants
@casperdcl @0x2b3bfa0 @DavidGOrtega @shcheklein