Skip to content

nushell: update to 0.7.0 and add stable features#48041

Closed
waldyrious wants to merge 2 commits intoHomebrew:masterfrom
waldyrious:patch-1
Closed

nushell: update to 0.7.0 and add stable features#48041
waldyrious wants to merge 2 commits intoHomebrew:masterfrom
waldyrious:patch-1

Conversation

@waldyrious
Copy link
Copy Markdown
Contributor

@waldyrious waldyrious commented Dec 18, 2019

This PR was originally created to add support for the starship prompt feature, but soon after creating it I realized that 0.7.0 had been released, so I updated the PR to upgrade the formula and enable the stable features (which include the starship prompt).

/cc @chenrui333 who has been involved in previous Nushell PRs.

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

This feature cannot be activated dynamically; it needs to enabled at compilation time.
@waldyrious
Copy link
Copy Markdown
Contributor Author

waldyrious commented Dec 18, 2019

Note: the test actually does not pass with the current diff, but it does if I make the feature optional:

  def install
    system "cargo", "install",
      *("--features starship-prompt" if build.with? "starship"),
      "--locked",
      "--root", prefix,
      "--path", "."
  end

Should I do that, or update the test to expect the starship prompt?

Edit: I fixed the test to match the output with the stable features enabled, which include the starship prompt.

@waldyrious
Copy link
Copy Markdown
Contributor Author

waldyrious commented Dec 19, 2019

I am also wondering if we should default to the stable features instead of singling out the starship prompt feature (which is included in the stable features set).

However, I was unable to get it to work with a similar syntax to the one currently in this PR (i.e. --features=stable), based on the instructions from the official README:

 def install
-    system "cargo", "install", "--locked", "--root", prefix, "--path", "."
+    system "cargo", "install", "--features", "stable", "--locked", "--root", prefix, "--path", "."
 end

Any thoughts about whether this is a better approach, and if so, how to make it work?

@waldyrious
Copy link
Copy Markdown
Contributor Author

Ah, I just now read the release notes for 0.7.0. Turns out the stable features set is a new addition, so it should be available if we update the version in this PR. I'll do so tomorrow.

@waldyrious
Copy link
Copy Markdown
Contributor Author

PR now updated to perform the 0.7.0 update as well as enable the stable features during build.

@waldyrious waldyrious changed the title nushell: add starship prompt feature nushell: update to 0.7.0 and add stable features Dec 19, 2019
@alebcay
Copy link
Copy Markdown
Member

alebcay commented Dec 23, 2019

@BrewTestBot test this please.

@chenrui333
Copy link
Copy Markdown
Member

lgtm, thanks @waldyrious!!

@lock lock bot added the outdated PR was locked due to age label Jan 24, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Jan 24, 2020
@waldyrious waldyrious deleted the patch-1 branch January 24, 2020 17:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

outdated PR was locked due to age

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants