Skip to content

Conversation

@andreaangiolillo
Copy link
Collaborator

@andreaangiolillo andreaangiolillo commented Feb 28, 2022

Proposed changes

Jira ticket: CLOUDP-112242

Checklist

  • I have signed the MongoDB CLA
  • I have added tests that prove my fix is effective or that my feature works
  • I have added any necessary documentation in document requirements section listed in CONTRIBUTING.md (if appropriate)
  • I have addressed the @mongodb/docs-cloud-team comments (if appropriate)
  • I have updated e2e/E2E-TESTS.md (if an e2e test has been added)
  • I have run make fmt and formatted my code

Further comments

This PR updates the README.MD to include atlascli. The new README is available here https://github.com/mongodb/mongocli/tree/CLOUDP-112242.
This PR will be merged once we have pre-released atlascli and renamed the repository to mongodb-atlas-cli

Next Step:

  • Pre-releasing atlascli v0.0.1
  • Renaming the github repository to mongodb-atlas-cli
  • Merging this PR
  • Releasing atlascli v1.0.0
  • Updating again the atlascli.md to add documentation links

@andreaangiolillo andreaangiolillo marked this pull request as ready for review March 1, 2022 14:17
blva
blva previously approved these changes Mar 1, 2022
@andreaangiolillo andreaangiolillo requested a review from gssbzn March 1, 2022 17:16
Copy link
Contributor

@gssbzn gssbzn left a comment

Choose a reason for hiding this comment

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

not sure what your end goal was with this change but if it was to expose Atlas CLI at least for me it's not working as expected.
The README it's becoming too much to parse and too much to scroll if I'm only interested in atlas cli.
Some alternatives to consider.

  1. Create a mongocli.md and a atlascli.md REAMEs and link to them from the main README
  2. Delete all content that exists somewhere else and leave only the essential, how to install, login, usage, link all to public docs, requirements about go and install from souce exist in the contributing file can be deleted. Probably also delete both gifs. TL;DR leave the readme a barebone of what the CLIs are link to publci docs for anything else

Copy link
Contributor

@JuliaMongo JuliaMongo left a comment

Choose a reason for hiding this comment

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

LGTM for copy review with these comments.

Copy link
Contributor

@gssbzn gssbzn left a comment

Choose a reason for hiding this comment

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

Thanks for the split of files, I think it makes a difference on organising the instructions, left some small comments to address before merging

@andreaangiolillo andreaangiolillo requested a review from gssbzn March 2, 2022 17:30
gssbzn
gssbzn previously approved these changes Mar 3, 2022
Copy link
Contributor

@gssbzn gssbzn left a comment

Choose a reason for hiding this comment

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

LGTM thanks for those changes

Copy link

@dianchenghu dianchenghu left a comment

Choose a reason for hiding this comment

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

Add some copy suggestions

gssbzn
gssbzn previously approved these changes Mar 4, 2022
Copy link
Contributor

@gssbzn gssbzn left a comment

Choose a reason for hiding this comment

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

LGTM

sarahsimpers
sarahsimpers previously approved these changes Mar 4, 2022
Copy link
Contributor

@sarahsimpers sarahsimpers left a comment

Choose a reason for hiding this comment

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

LGTM % one optional nit for language.

Copy link

@dianchenghu dianchenghu left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

@andreaangiolillo andreaangiolillo requested a review from gssbzn March 7, 2022 08:33
@andreaangiolillo andreaangiolillo merged commit 952f73b into master Mar 7, 2022
@andreaangiolillo andreaangiolillo deleted the CLOUDP-112242 branch March 7, 2022 16:37
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.

8 participants