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

fix: Make sure Rover works in non-git repos #282

Merged
merged 5 commits into from
Feb 22, 2021
Merged

fix: Make sure Rover works in non-git repos #282

merged 5 commits into from
Feb 22, 2021

Conversation

JakeDawkins
Copy link
Contributor

@JakeDawkins JakeDawkins commented Feb 18, 2021 •
edited
Loading

Fixes #271

I'd love stylistic feedback here if anyone has any! I tried to walk the line of easy to understand code vs concise code lol

@JakeDawkins JakeDawkins changed the title Makes sure Rover works in non-git repos feat: Make sure Rover works in non-git repos Feb 18, 2021
@JakeDawkins JakeDawkins added changelog - fix feature 🎉 new commands, flags, functionality, and improved error messages labels Feb 18, 2021
@JakeDawkins JakeDawkins changed the title feat: Make sure Rover works in non-git repos fix: Make sure Rover works in non-git repos Feb 19, 2021
lrlna
lrlna approved these changes Feb 19, 2021
View reviewed changes
Copy link
Member

@lrlna lrlna left a comment

Choose a reason for hiding this comment

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

Added a suggestion on the style, but this looks good!

src/utils/git.rs Outdated Show resolved Hide resolved
JakeDawkins and others added 2 commits February 19, 2021 09:29
@JakeDawkins @lrlna
Update src/utils/git.rs
2d73df4 
Co-authored-by: Irina Shestak <shestak.irina@gmail.com>
@JakeDawkins
Merge branch 'main' into jake/git-it-together
acea30d
@EverlastingBugstopper EverlastingBugstopper added this to the 0.0.2 milestone Feb 19, 2021
@lrlna
Copy link
Member

lrlna commented Feb 22, 2021

I am unable to make a suggestion for this but line 42-48 in git.rs can be removed, since we are returning Ok() from the match statement, and then the lints should pass and we can merge this in \o/

@JakeDawkins JakeDawkins merged commit ec435a5 into main Feb 22, 2021
@JakeDawkins JakeDawkins deleted the jake/git-it-together branch February 22, 2021 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lrlna lrlna lrlna approved these changes

@EverlastingBugstopper EverlastingBugstopper Awaiting requested review from EverlastingBugstopper

Assignees
No one assigned
Labels
feature 🎉 new commands, flags, functionality, and improved error messages
Projects
None yet
Milestone
February 23
Development

Successfully merging this pull request may close these issues.

subgraph push seems to require operating in a git repo.
3 participants
@JakeDawkins @lrlna @EverlastingBugstopper