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

goal: allow for relative dataDir via -d cmd option #5067

Merged
merged 1 commit into from Jan 27, 2023
Merged

goal: allow for relative dataDir via -d cmd option #5067

merged 1 commit into from Jan 27, 2023

Conversation

ghost
Copy link

@ghost ghost commented Jan 26, 2023

Post Merge Complaint

[COMPLAINT] Is the Algorand Inc. aware of the heavily dysfunctional Algorand Foundation?

Summary

Allow relative data-dir, as described within #589

Test Plan

existent tests pass, added some more tests

go test -v ./cmd/goal

Follow-up

#5073

@CLAassistant
Copy link

CLAassistant commented Jan 26, 2023

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Jan 26, 2023

Codecov Report

Merging #5067 (300cc10) into master (427a119) will increase coverage by 0.02%.
The diff coverage is 60.00%.

@@            Coverage Diff             @@
##           master    #5067      +/-   ##
==========================================
+ Coverage   53.56%   53.58%   +0.02%     
==========================================
  Files         430      430              
  Lines       54091    54094       +3     
==========================================
+ Hits        28972    28985      +13     
+ Misses      22873    22860      -13     
- Partials     2246     2249       +3     
Impacted Files Coverage Δ
cmd/goal/commands.go 12.72% <60.00%> (+0.22%) ⬆️
ledger/blockqueue.go 81.18% <0.00%> (-1.08%) ⬇️
catchup/peerSelector.go 98.95% <0.00%> (-1.05%) ⬇️
ledger/testing/randomAccounts.go 56.26% <0.00%> (-0.62%) ⬇️
ledger/catchpointtracker.go 58.49% <0.00%> (ø)
network/wsNetwork.go 64.92% <0.00%> (+0.17%) ⬆️
catchup/service.go 70.04% <0.00%> (+0.24%) ⬆️
ledger/acctupdates.go 69.60% <0.00%> (+0.36%) ⬆️
cmd/tealdbg/debugger.go 72.44% <0.00%> (+0.78%) ⬆️
network/wsPeer.go 68.79% <0.00%> (+2.36%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ghost ghost changed the title allow for relative dataDir via -d, closes #589 Bug-Fix: allow for relative dataDir via -d, closes #589 Jan 26, 2023
@ghost
Copy link
Author

ghost commented Jan 26, 2023

@algorandskiy , the Ci complains...

Label error. Requires exactly 1 of: New Feature, Enhancement, Bug-Fix, Not-Yet-Enabled, Skip-Release-Notes. Found: user-facing, external contribution

Copy link
Contributor

@algorandskiy algorandskiy left a comment

Choose a reason for hiding this comment

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

I'd make resolveDataDir a function taking dirs as its argument, and update cmd/algocfg/datadir.go as well, and probably move it to cmd/utils. But this also LGTM

@ghost
Copy link
Author

ghost commented Jan 27, 2023

I'd make resolveDataDir a function taking dirs as its argument, and update cmd/algocfg/datadir.go as well, and probably move it to cmd/utils.

I did not notice the code-duplication, there are even more resolveDataDir and other DataDir related duplicates.

Would prefer to handle them in a separate follow-up issue/PR (see #5068).

But this also LGTM

Ty! (btw: fascinating fast PR processing!)

@algorandskiy algorandskiy changed the title Bug-Fix: allow for relative dataDir via -d, closes #589 goal: allow for relative dataDir via -d Jan 27, 2023
@algorandskiy algorandskiy changed the title goal: allow for relative dataDir via -d goal: allow for relative dataDir via -d cmd option Jan 27, 2023
@algorandskiy
Copy link
Contributor

closes #589

@algorandskiy algorandskiy merged commit ec75266 into algorand:master Jan 27, 2023
@ghost
Copy link
Author

ghost commented Jan 28, 2023

@algorandskiy , somehow the github auto-close-issue did not trigger, still open: #589

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants