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

Import go-log directly (except in GPBFT) and remove Logging #427

Merged
merged 5 commits into from
Jul 8, 2024

Conversation

Stebalien
Copy link
Member

It was causing circular import issues (which could have been fixed by moving it, I guess) but, more importantly, the abstraction really wasn't pulling its weight. We're keeping it out of GPBFT itself but our other packages already depend on things like go-datastore and go-libp2p, so depending on go-log doesn't really matter.

@Stebalien Stebalien requested a review from Kubuxu July 8, 2024 09:51
@Stebalien Stebalien enabled auto-merge July 8, 2024 09:56
host.go Outdated Show resolved Hide resolved
Copy link

codecov bot commented Jul 8, 2024

Codecov Report

Attention: Patch coverage is 40.54054% with 22 lines in your changes missing coverage. Please review.

Project coverage is 74.36%. Comparing base (8df83eb) to head (2d43052).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #427      +/-   ##
==========================================
- Coverage   74.77%   74.36%   -0.42%     
==========================================
  Files          41       42       +1     
  Lines        3719     3713       -6     
==========================================
- Hits         2781     2761      -20     
- Misses        657      665       +8     
- Partials      281      287       +6     
Files Coverage Δ
logging.go 100.00% <100.00%> (ø)
certexchange/polling/subscriber.go 78.88% <75.00%> (ø)
cmd/f3/run.go 0.00% <0.00%> (ø)
f3.go 70.46% <50.00%> (-3.54%) ⬇️
certexchange/client.go 66.66% <33.33%> (+1.51%) ⬆️
certexchange/server.go 59.01% <0.00%> (-9.84%) ⬇️
host.go 62.08% <42.85%> (-3.31%) ⬇️

... and 2 files with indirect coverage changes

It was causing circular import issues (which could have been fixed by
moving it, I guess) but, more importantly, the abstraction really wasn't
pulling its weight. We're keeping it out of GPBFT itself but our other
packages already depend on things like go-datastore and go-libp2p, so
depending on go-log doesn't really matter.
logging.go Outdated Show resolved Hide resolved
cmd/f3/run.go Outdated Show resolved Hide resolved
certexchange/server.go Outdated Show resolved Hide resolved
@Stebalien Stebalien added this pull request to the merge queue Jul 8, 2024
Merged via the queue into main with commit 3b6ea9e Jul 8, 2024
11 of 13 checks passed
@Stebalien Stebalien deleted the steb/remove-logging-param branch July 8, 2024 10:28
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