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

refactor: No error return for print #1375

Merged
merged 1 commit into from
May 7, 2024

Conversation

TerryHowe
Copy link
Member

What this PR does / why we need it:

I don't think we should consider print errors a problem most of the time and we should manage the output of the error to print only once.

@TerryHowe TerryHowe changed the title refacor: No return for print refactor: No error return for print May 6, 2024
@TerryHowe TerryHowe force-pushed the no-return-on-error branch from 6c66d5a to 070b7e8 Compare May 6, 2024 14:47
Copy link

codecov bot commented May 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.81%. Comparing base (101cf17) to head (2acae64).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1375      +/-   ##
==========================================
+ Coverage   84.75%   84.81%   +0.06%     
==========================================
  Files         104      104              
  Lines        3712     3715       +3     
==========================================
+ Hits         3146     3151       +5     
+ Misses        338      336       -2     
  Partials      228      228              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

cmd/oras/internal/display/status/print.go Outdated Show resolved Hide resolved
cmd/oras/internal/display/status/deprecated.go Outdated Show resolved Hide resolved
@qweeah
Copy link
Contributor

qweeah commented May 7, 2024

If && p.errored == nil is not required, then errored can be removed from the printer like:

// Println prints objects concurrent-safely with newline.
func (p *Printer) Println(a ...any) error {
	p.lock.Lock()
	defer p.lock.Unlock()
	_, err := fmt.Fprintln(p.out, a...)
	if err != nil  {
		err = fmt.Errorf("display output error: %w", err)
		_, _ = fmt.Fprint(os.Stderr, err)
	}
        return err
}

@TerryHowe TerryHowe force-pushed the no-return-on-error branch from edc4efe to 9e5c050 Compare May 7, 2024 11:43
@TerryHowe
Copy link
Member Author

Last commit only contains improved test errors

@TerryHowe TerryHowe force-pushed the no-return-on-error branch 2 times, most recently from daa358e to 00d4ada Compare May 7, 2024 12:40
@TerryHowe TerryHowe force-pushed the no-return-on-error branch from 00d4ada to 786af10 Compare May 7, 2024 13:32
@TerryHowe TerryHowe force-pushed the no-return-on-error branch from 786af10 to 2acae64 Compare May 7, 2024 13:44
Copy link
Contributor

@qweeah qweeah left a comment

Choose a reason for hiding this comment

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

LGTM

@TerryHowe TerryHowe merged commit a758bdb into oras-project:main May 7, 2024
8 checks passed
FeynmanZhou pushed a commit to FeynmanZhou/oras that referenced this pull request May 11, 2024
@TerryHowe TerryHowe deleted the no-return-on-error branch May 27, 2024 14:35
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.

2 participants