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

chore: clean&unify --data-dir argument to fedimintd and fedimint-cli #2063

Merged
merged 1 commit into from
Mar 30, 2023

Conversation

dpc
Copy link
Contributor

@dpc dpc commented Mar 29, 2023

  • Make it possible to pass by env variable
  • Make it always explicit, not positional
  • Use the same name (keep an alias for easier transation)

@codecov-commenter
Copy link

codecov-commenter commented Mar 29, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.02 ⚠️

Comparison is base (3ddb64f) 62.76% compared to head (51562d1) 62.74%.

❗ Current head 51562d1 differs from pull request most recent head 7424850. Consider uploading reports for the commit 7424850 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2063       /-   ##
==========================================
- Coverage   62.76%   62.74%   -0.02%     
==========================================
  Files         144      144              
  Lines       29266    29267        1     
==========================================
- Hits        18369    18365       -4     
- Misses      10897    10902        5     
Impacted Files Coverage Δ
client/cli/src/lib.rs 0.18% <0.00%> (ø)
fedimint-testing/src/bin/fixtures.rs 0.24% <0.00%> (-0.01%) ⬇️
fedimintd/src/fedimintd.rs 0.00% <ø> (ø)

... and 10 files with indirect coverage changes

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 in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@dpc dpc marked this pull request as ready for review March 29, 2023 21:27
client/cli/src/lib.rs Outdated Show resolved Hide resolved
@dpc dpc force-pushed the unify-data-dir branch from 0ca4766 to b052e8e Compare March 29, 2023 21:32
justinmoon
justinmoon previously approved these changes Mar 30, 2023
@@ -70,7 70,7 @@ mkdir -p $FM_GATEWAY_DATA_DIR
export FM_LIGHTNING_CLI="lightning-cli --network regtest --lightning-dir=$FM_CLN_DIR"
export FM_LNCLI="lncli -n regtest --lnddir=$FM_LND_DIR --rpcserver=localhost:11009"
export FM_BTC_CLIENT="bitcoin-cli -regtest -rpcuser=bitcoin -rpcpassword=bitcoin"
export FM_MINT_CLIENT="$FM_BIN_DIR/fedimint-cli --workdir $FM_CFG_DIR"
export FM_MINT_CLIENT="$FM_BIN_DIR/fedimint-cli --data-dir $FM_CFG_DIR"
Copy link
Member

Choose a reason for hiding this comment

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

should we replace this with env variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe sometime. But for now I think making it as much 1:1 change is best.

codecov.yml Outdated Show resolved Hide resolved
* Make it possible to pass by env variable
* Make it always explicit, not positional
* Use the same name (keep an alias for easier transation)
@justinmoon
Copy link
Contributor

So our target for the patch (codecov/patch) is the coverage rate of the entire project?

Also, I was expecting that codecov/project to pass so long as the coverage rate didn't go down by 1% ... https://github.com/fedimint/fedimint/pull/2065/files#r1152659761

@justinmoon justinmoon merged commit d4048da into fedimint:master Mar 30, 2023
@dpc dpc deleted the unify-data-dir branch November 12, 2024 04:39
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.

5 participants