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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add a julia module #1030

Merged
merged 3 commits into from
Apr 3, 2020
Merged

feat: Add a julia module #1030

merged 3 commits into from
Apr 3, 2020

Conversation

cappyzawa
Copy link
Member

Description

Motivation and Context

Closes #

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Screenshots (if appropriate):

Screen Shot 2020-03-25 at 11 32 57

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.

@cappyzawa cappyzawa changed the title feat: add a julia module feat: Add a julia module Mar 25, 2020
@cappyzawa cappyzawa requested a review from a team March 25, 2020 02:42
docs/config/README.md Outdated Show resolved Hide resolved
Co-Authored-By: Thomas O'Donnell <andytom@users.noreply.github.com>
Copy link
Contributor

@chipbuster chipbuster left a comment

Choose a reason for hiding this comment

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

Holy cow the number of things we need to change to add a new module is getting high. We should probably make a list somewhere...

As an aside, do you think the "therefore" emoji (∴) might be an appropriate default symbol? It's not exactly the julia logo, but it seems to be used in at least some places that are common (e.g. it's pretty similar to the favicon used on the Julia website)

image

@cappyzawa
Copy link
Member Author

@chipbuster

Holy cow the number of things we need to change to add a new module is getting high. We should probably make a list somewhere...

I think so.

As an aside, do you think the "therefore" emoji (∴) might be an appropriate default symbol?

That's nice!
I was having trouble finding a good symbol.

@heyrict
Copy link
Member

heyrict commented Mar 27, 2020

It seems that julia uses Project.toml to store version info. What do you think about adding some code to package module to display the version?

@cappyzawa
Copy link
Member Author

It seems that julia uses Project.toml to store version info. What do you think about adding some code to package module to display the version?

It's sound good.
However, I want to implement it in another PR because it is not included in the julia module.

Copy link
Member

@andytom andytom left a comment

Choose a reason for hiding this comment

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

LGTM

@andytom andytom merged commit dba3467 into starship:master Apr 3, 2020
# ~/.config/starship.toml

[julia]
symbol = "👸 "

Choose a reason for hiding this comment

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

I believe this example may not be compliant with Julia's community standards:

do not sexualize the term "Julia" or any other aspects of the project. While "Julia" is a female name in many parts of the world, the programming language is not a person and does not have a gender.

I'd suggest using something different, e.g. "Mathematical Bold Capital J" (U+1D409):

symbol = "𝐉 "

or "Mathematical Sans-serif Bold Small J" (U+1D5F7), which looks similar to the j in Julia's logo:

symbol = "𝗷 "

WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, it would probably be best if we changed it.
I like both your suggestions, @waldyrious. Another option if the "Therefore" symbol, present in the Julia logotype and favicon: or

Choose a reason for hiding this comment

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

The therefore symbol is actually already the default :) if I understood the code correctly, this was an example of customizing the symbol. However, the second symbol you listed is an even better match to the Julia logo! I'd suggest replacing the current default with it and using one of the J symbols I proposed as an example of customization.

Copy link
Member

Choose a reason for hiding this comment

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

The therefore symbol is actually already the default :)

Ah right. My bad. 🤦‍♂
This is what happens when I swoop into issues without reading the context. 😅

However, the second symbol you listed is an even better match to the Julia logo! I'd suggest replacing the current default with it and using one of the J symbols I proposed as an example of customization.

Sounds good to me. 👍
Would you be open to creating a PR for the change, Waldir?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh.. I didn't know https://julialang.org/community/standards/ 🙇

I will fix this as soon as possible.

dagbrown pushed a commit to dagbrown/starship that referenced this pull request Oct 22, 2021
* add a julia module

* Update docs/config/README.md

Co-Authored-By: Thomas O'Donnell <andytom@users.noreply.github.com>

* fix based on starship#1030 (review)

Co-authored-by: Thomas O'Donnell <andytom@users.noreply.github.com>
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.

None yet

6 participants