Skip to content

cmds/core/grep: remove unused struct and rearrange fields#2714

Merged
binjip978 merged 3 commits into
u-root:mainfrom
binjip978:remove-dead-struct
Jul 13, 2023
Merged

cmds/core/grep: remove unused struct and rearrange fields#2714
binjip978 merged 3 commits into
u-root:mainfrom
binjip978:remove-dead-struct

Conversation

@binjip978

@binjip978 binjip978 commented Jul 7, 2023

Copy link
Copy Markdown
Contributor

No description provided.

@binjip978
cmds/core/grep: remove unused struct and rearrange fields
48c2abe 
Signed-off-by: Siarhiej Siemianczuk <pdp.eleven11@gmail.com>
@binjip978 binjip978 added the Awaiting reviewer Waiting for a reviewer. label Jul 7, 2023
rminnich
rminnich reviewed Jul 12, 2023
View reviewed changes
Comment thread cmds/core/grep/grep.go Outdated
@rminnich rminnich added Awaiting author Waiting for new changes or feedback for author. and removed Awaiting reviewer Waiting for a reviewer. labels Jul 12, 2023
@binjip978
remove exprB and move strings back
fee1565 
Signed-off-by: Siarhiej Siemianczuk <pdp.eleven11@gmail.com>
@codecov

codecov Bot commented Jul 12, 2023 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.17 ⚠️

Comparison is base (b3926bb) 75.54% compared to head (46cbd17) 75.38%.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2714      +/-   ##
==========================================
- Coverage   75.54%   75.38%   -0.17%     
==========================================
  Files         415      415              
  Lines       42222    42221       -1     
==========================================
- Hits        31897    31828      -69     
- Misses      10325    10393      +68
Impacted Files Coverage Δ
cmds/core/grep/grep.go 91.96% <100.00%> (-0.08%) ⬇️

... and 5 files with indirect coverage changes

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

@binjip978 binjip978 requested a review from rminnich July 12, 2023 17:20
@binjip978 binjip978 added Awaiting reviewer Waiting for a reviewer. and removed Awaiting author Waiting for new changes or feedback for author. labels Jul 12, 2023
rminnich
rminnich reviewed Jul 12, 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 realize this is not part of the PR, but do you think we can move mainParams to main()
If so, this makes it easier to use this code in tinygo/tamago

@binjip978

binjip978 commented Jul 12, 2023

Copy link
Copy Markdown
Contributor Author

realize this is not part of the PR, but do you think we can move mainParams to main()
If so, this makes it easier to use this code in tinygo/tamago

The problem with moving mainParams to main is that we have _plan9 and _linux files. So they use different parameters for quite (silent) mode. So they need this global state mainParams. But may be we get rid both of them and support quite mode with -s and -q for any operating system. In this case everyone will be happy and we can move params into main. If it's ok? I can do separate PR for this.

@rminnich

rminnich commented Jul 13, 2023

Copy link
Copy Markdown
Member

let's delete the _plan9 file and go with your idea.

@rminnich rminnich added Awaiting author Waiting for new changes or feedback for author. and removed Awaiting reviewer Waiting for a reviewer. labels Jul 13, 2023
@binjip978 binjip978 merged commit cd15f19 into u-root:main Jul 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rminnich rminnich rminnich approved these changes

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.

2 participants

@binjip978 @rminnich