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

feat(deno): create module #2565

Merged
merged 22 commits into from Apr 15, 2021
Merged

feat(deno): create module #2565

merged 22 commits into from Apr 15, 2021

Conversation

Milo123459
Copy link
Contributor

@Milo123459 Milo123459 commented Apr 8, 2021 •
edited

Description

Added a new deno module

Motivation and Context

Closes #

Screenshots (if appropriate):

img

How Has This Been Tested?

  • I have tested using MacOS
  • I have tested using Linux
  • I have tested using Windows

Checklist:

  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.

@Milo123459 Milo123459 changed the title Add deno module feat(deno): create moudle Apr 8, 2021
@Milo123459 Milo123459 changed the title feat(deno): create moudle feat(deno): create module Apr 8, 2021
@davidkna davidkna requested a review from a team April 8, 2021 18:57
@Milo123459
Copy link
Contributor Author

Currently, if Deno is not installed, it looks like this:
img

Is there something I need to do to make sure it doesn't look like this? Or can I just add a check and do a return None?

vladimyr
vladimyr requested changes Apr 9, 2021
View reviewed changes
Copy link
Member

@vladimyr vladimyr left a comment

Choose a reason for hiding this comment

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

It looks good overall 👍
Please address the comments I've left you and add tests for the remaining two files that trigger module display.

src/utils.rs Outdated Show resolved Hide resolved
src/modules/deno.rs Outdated Show resolved Hide resolved
src/modules/deno.rs Outdated Show resolved Hide resolved
src/modules/deno.rs Outdated Show resolved Hide resolved
src/modules/deno.rs Outdated Show resolved Hide resolved
Milo123459 and others added 10 commits April 9, 2021 08:44
@Milo123459 @vladimyr
Update src/utils.rs
f8a6224 
Co-authored-by: Dario Vladović <d.vladimyr@gmail.com>
@Milo123459 @vladimyr
Update src/modules/deno.rs
0504d8a 
Co-authored-by: Dario Vladović <d.vladimyr@gmail.com>
@Milo123459 @vladimyr
Update src/modules/deno.rs
c589d07 
Co-authored-by: Dario Vladović <d.vladimyr@gmail.com>
@Milo123459 @vladimyr
Update src/modules/deno.rs
cdb72f7 
Co-authored-by: Dario Vladović <d.vladimyr@gmail.com>
@Milo123459 @vladimyr
Update src/modules/deno.rs
ec6504d 
Co-authored-by: Dario Vladović <d.vladimyr@gmail.com>
@Milo123459
run rust fmt
4d342cd
@Milo123459
Use deno -V
7f16a01
@Milo123459
fmt
0169237
@Milo123459
Fix deno module
52b86c7
@Milo123459
do clippy
40c4622
@wyze
Copy link
Member

wyze commented Apr 9, 2021

@Milo123459 You could move the version check up, right below the is_deno_project, variable. Then you can return None if the version comes back as None.

@Milo123459
Copy link
Contributor Author

@Milo123459 You could move the version check up, right below the is_deno_project, variable. Then you can return None if the version comes back as None.

I've spoken to people in the discord and they said leave it as is. Thanks though.

davidkna
davidkna requested changes Apr 9, 2021
View reviewed changes
src/configs/deno.rs Show resolved Hide resolved
src/modules/deno.rs Show resolved Hide resolved
src/modules/deno.rs Outdated Show resolved Hide resolved
docs/config/README.md Outdated Show resolved Hide resolved
Milo123459 and others added 8 commits April 9, 2021 14:09
@Milo123459 @davidkna
Update src/configs/deno.rs
f6c70f8 
Co-authored-by: David Knaack <davidkna@users.noreply.github.com>
@Milo123459 @davidkna
Update src/modules/deno.rs
2f98bbb 
Co-authored-by: David Knaack <davidkna@users.noreply.github.com>
@Milo123459 @davidkna
Update docs/config/README.md
a6bc4e5 
Co-authored-by: David Knaack <davidkna@users.noreply.github.com>
@Milo123459
Fix test and docs
284bda0
@Milo123459
Remove unused code
369b802
@Milo123459
Fix conflicts
af71eca
@Milo123459
fmt
96d6bf6
@Milo123459
update configs
7dc249d
@vladimyr
Copy link
Member

... and add tests for the remaining two files that trigger module display.

@Milo123459 Please apply the following patch: https://github.com/vladimyr/starship/commit/eb09433.patch

@Milo123459
Copy link
Contributor Author

... and add tests for the remaining two files that trigger module display.

@Milo123459 Please apply the following patch: vladimyr/starship@eb09433.patch

Oh my bad, I forgot. Adding now.

@Milo123459
Copy link
Contributor Author

Added those tests @vladimyr

vladimyr
vladimyr requested changes Apr 10, 2021
View reviewed changes
docs/config/README.md Outdated Show resolved Hide resolved
@Milo123459 @vladimyr
Update docs/config/README.md
e8ec896 
Co-authored-by: Dario Vladović <d.vladimyr@gmail.com>
vladimyr
vladimyr approved these changes Apr 10, 2021
View reviewed changes
Copy link
Member

@vladimyr vladimyr left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@davidkna davidkna merged commit 3cb15ab into starship:master Apr 15, 2021
@davidkna
Copy link
Member

Thanks for the contribution @Milo123459 and thanks for reviewing @vladimyr.

@kidonng kidonng mentioned this pull request Apr 22, 2021
Closed
DavidSmith166 pushed a commit to DavidSmith166/starship that referenced this pull request Apr 24, 2021
@Milo123459 @vladimyr
@davidkna @DavidSmith166
feat(deno): create module (starship#2565)
4276946 
* Add deno module

* Update docs

* Update src/utils.rs

Co-authored-by: Dario Vladović <d.vladimyr@gmail.com>

* Update src/modules/deno.rs

Co-authored-by: Dario Vladović <d.vladimyr@gmail.com>

* Update src/modules/deno.rs

Co-authored-by: Dario Vladović <d.vladimyr@gmail.com>

* Update src/modules/deno.rs

Co-authored-by: Dario Vladović <d.vladimyr@gmail.com>

* Update src/modules/deno.rs

Co-authored-by: Dario Vladović <d.vladimyr@gmail.com>

* run rust fmt

* Use deno -V

* fmt

* Fix deno module

* do clippy

* Update src/configs/deno.rs

Co-authored-by: David Knaack <davidkna@users.noreply.github.com>

* Update src/modules/deno.rs

Co-authored-by: David Knaack <davidkna@users.noreply.github.com>

* Update docs/config/README.md

Co-authored-by: David Knaack <davidkna@users.noreply.github.com>

* Fix test and docs

* Remove unused code

* fmt

* update configs

* Add more tests

* Update docs/config/README.md

Co-authored-by: Dario Vladović <d.vladimyr@gmail.com>

Co-authored-by: Dario Vladović <d.vladimyr@gmail.com>
Co-authored-by: David Knaack <davidkna@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidkna davidkna davidkna requested changes

@vladimyr vladimyr vladimyr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@Milo123459 @wyze @vladimyr @davidkna