Skip to content

Move gosh to cmds/core#2592

Closed
jenstopp wants to merge 2 commits into
mainfrom
gosh
Closed

Move gosh to cmds/core#2592
jenstopp wants to merge 2 commits into
mainfrom
gosh

Conversation

@jenstopp

@jenstopp jenstopp commented Jan 5, 2023

Copy link
Copy Markdown
Member

With recent work in #2554 #2555 and #2569 gosh isn't experimental anymore.
Consideration if it should be the default shell, here: #2591

Move gosh to cmds/core
f9a5e36 
Tab completion has been fixed. The shell is stable, tested and
ready to by use.

Signed-off-by: Jens Drenhaus <jens.drenhaus@9elements.com>
@MDr164

MDr164 commented Jan 5, 2023

Copy link
Copy Markdown
Contributor

CI fails currently because our used shell parser doesn't seem to support Plan9 yet. Every other target OS should work fine, as we got general Unix and Windows support. Either blacklist the package using build constraints for Plan9 or add Plan9 support to the shell parser library.

@jenstopp

jenstopp commented Jan 5, 2023

Copy link
Copy Markdown
Member Author

CI fails currently because our used shell parser doesn't seem to support Plan9 yet. Every other target OS should work fine, as we got general Unix and Windows support. Either blacklist the package using build constraints for Plan9 or add Plan9 support to the shell parser library.

We should address this issue when considering gosh as default shell #2591.

cmds/core/gosh: Add build constraints
22076e3 
Signed-off-by: Jens Drenhaus <jens.drenhaus@9elements.com>
@codecov

codecov Bot commented Jan 5, 2023 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Base: 74.75% // Head: 74.75% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (22076e3) compared to base (ff9c888).
Patch has no changes to coverable lines.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2592      +/-   ##
==========================================
- Coverage   74.75%   74.75%   -0.01%     
==========================================
  Files         408      408              
  Lines       41335    41335              
==========================================
- Hits        30900    30899       -1     
- Misses      10435    10436       +1
Impacted Files Coverage Δ
cmds/core/gosh/completer.go 100.00% <ø> (ø)
cmds/core/gosh/gosh.go 54.00% <ø> (ø)
cmds/core/mv/mv.go 77.27% <0.00%> (-2.28%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

rminnich
rminnich requested changes Jan 6, 2023
View reviewed changes

@rminnich rminnich left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I need to remind you -- you aren't following the rules for submitting PRs, listed here:
https://github.com/u-root/u-root/blob/main/CONTRIBUTING.md
"Fork the project on GitHub, work in your fork and in branches, push these to your GitHub fork, and when ready, do a GitHub pull requests against https://github.com/u-root/u-root."

Can you please redo this PR, following the rules? Thanks.

@rminnich

rminnich commented Jan 6, 2023

Copy link
Copy Markdown
Member

u-root has no plan 9 users at this point, so it may be time to remove all plan 9 support and artifacts.

Rather than scattering plan 9 build constraints everywhere, I'd be fine with removing plan 9 entirely; it's not clear it will be supported by Go for much longer anyway.

@rminnich rminnich added the Awaiting author Waiting for new changes or feedback for author. label Jan 6, 2023
@jenstopp

jenstopp commented Jan 8, 2023

Copy link
Copy Markdown
Member Author

I need to remind you -- you aren't following the rules for submitting PRs, listed here: https://github.com/u-root/u-root/blob/main/CONTRIBUTING.md "Fork the project on GitHub, work in your fork and in branches, push these to your GitHub fork, and when ready, do a GitHub pull requests against https://github.com/u-root/u-root."

Can you please redo this PR, following the rules? Thanks.

Sure. #2598

@jenstopp jenstopp closed this Jan 8, 2023
@jenstopp jenstopp mentioned this pull request Jan 9, 2023
Open
@hugelgupf hugelgupf deleted the gosh branch March 4, 2024 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rminnich rminnich rminnich requested changes

@walterchris walterchris Awaiting requested review from walterchris

@MDr164 MDr164 Awaiting requested review from MDr164

Assignees

No one assigned

Labels

Awaiting author Waiting for new changes or feedback for author.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jenstopp @MDr164 @rminnich