Consolidate Duplicate Server appConfig and Config Structure #391
+450
−523
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
louisinger
requested changes
Dec 3, 2024
There was a problem hiding this comment.
Beside FromConfig, LGTM, thanks for the contribution 🙏
cc @altafan
aruokhai
force-pushed
the
consolidate-server-conf
branch
from
3f8b261 to
e3809b8
Compare
louisinger
approved these changes
Dec 9, 2024
altafan
approved these changes
Dec 9, 2024
bordalix
reviewed
Dec 9, 2024
|
Please merge master in your branch, @bordalix should have been addressed already |
aruokhai
force-pushed
the
consolidate-server-conf
branch
from
e3809b8 to
a922e41
Compare
bordalix
reviewed
Dec 13, 2024
bordalix
reviewed
Dec 13, 2024
| ) | ||
| } | ||
|
|
||
| if c.BoardingExitDelay.Type == common.LocktimeTypeBlock { |
There was a problem hiding this comment.
Same here, but with BoardingExitDelay
louisinger
requested changes
Dec 19, 2024
server/internal/config/config.go
Outdated
| @@ -216,3 +272,343 @@ func getNetwork() (common.Network, error) { | |||
| return common.Network{}, fmt.Errorf("unknown network %s", viper.GetString(Network)) | |||
| } | |||
| } | |||
|
|
|||
| func DetermineLocktimeType(locktime int64) common.Locktime { | |||
There was a problem hiding this comment.
Suggested change
| func DetermineLocktimeType(locktime int64) common.Locktime { | |
| func determineLocktimeType(locktime int64) common.Locktime { |
no need to be exported, right ? cc @altafan
aruokhai
force-pushed
the
consolidate-server-conf
branch
2 times, most recently
from
644a654 to
4657e95
Compare
louisinger
approved these changes
Jan 2, 2025
aruokhai
force-pushed
the
consolidate-server-conf
branch
from
4657e95 to
cb8b760
Compare
louisinger
requested changes
Jan 7, 2025
louisinger
approved these changes
Jan 15, 2025
aruokhai
force-pushed
the
consolidate-server-conf
branch
from
a8ef1d1 to
7868f23
Compare
|
@aruokhai please sync again with master and include the changes made to config/app-config 🙏 |
aruokhai
force-pushed
the
consolidate-server-conf
branch
2 times, most recently
from
35dc715 to
2b3aff7
Compare
aruokhai
force-pushed
the
consolidate-server-conf
branch
from
2b3aff7 to
c489ede
Compare
fix, do not export private function fix rebased
aruokhai
force-pushed
the
consolidate-server-conf
branch
from
c489ede to
fd7339b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
A duplicate configuration struct in the Server directory complicates coordination. By consolidating it into a single Config structure, the configuration flow becomes more streamlined and efficient.
Scope
Non-critical, as it only affects Config parsing and usage.
closes #390