Skip to content

Rewrite the configuration package (very basic for now) - #98

Merged
jcxldn merged 22 commits into
feat/jcx/major/modules-deffrom
feat/modules-pr/config
Mar 17, 2021
Merged

Rewrite the configuration package (very basic for now)#98
jcxldn merged 22 commits into
feat/jcx/major/modules-deffrom
feat/modules-pr/config

Conversation

@jcxldn

@jcxldnjcxldn commented Mar 5, 2021•
edited
Loading

Copy link
Copy Markdown
Member

No description provided.

@jcxldnjcxldn added dependencies Pull requests that update a dependency file breaking p0 bug Very high priority bug (server crashing etc) labels Mar 5, 2021
@jcxldn
jcxldn requested a review from kokumajiMarch 5, 2021 12:44
@jcxldnjcxldn self-assigned this Mar 5, 2021
kokumaji
kokumaji approved these changes Mar 5, 2021
View reviewed changes

@kokumajikokumaji left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe also add something like YamlProvider#reload()? Other than that, lgtm 👍
Waiting for opinions from others

@kaylendog
kaylendog self-requested a review March 5, 2021 13:59
kaylendog
kaylendog requested changes Mar 5, 2021
View reviewed changes

@kaylendogkaylendog left a comment •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we move this into config? Better integrates with future plans for a configuration module. Otherwise LGTM :3

Comment threadcommon/src/main/java/com/dumbdogdiner/stickyapi/common/configuration/Configuration.java
@codecov

codecovBot commented Mar 5, 2021•
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #98 (f8f7306) into feat/jcx/major/modules-def (d322b65) will increase coverage by 11.20%.
The diff coverage is 73.33%.

Impacted file tree graph

@@ Coverage Diff @@## feat/jcx/major/modules-def #98 +/- ##
===============================================================
+ Coverage 23.19% 34.39% +11.20% 
===============================================================
Files 73 63 -10 Lines 3126 2532 -594 Branches 426 286 -140 ===============================================================
+ Hits 725 871 +146 + Misses 2355 1597 -758 - Partials 46 64 +18
Impacted FilesCoverage Δ
...umbdogdiner/stickyapi/bukkit/util/StartupUtil.java0.00% <0.00%> (ø)
...ogdiner/stickyapi/bungeecord/util/StartupUtil.java0.00% <0.00%> (ø)
...r/stickyapi/common/translation/LocaleProvider.java90.62% <ø> (+90.62%)⬆️
...i/common/configuration/providers/YamlProvider.java100.00% <100.00%> (ø)
...bdogdiner/stickyapi/common/translation/Locale.java80.00% <100.00%> (+80.00%)⬆️
...dumbdogdiner/stickyapi/common/util/StringUtil.java91.13% <0.00%> (-3.80%)⬇️
...iner/stickyapi/common/translation/Translation.java34.73% <0.00%> (+18.94%)⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d322b65...f8f7306. Read the comment docs.

@jcxldnjcxldn mentioned this pull request Mar 5, 2021
Merged
@jcxldn
jcxldnforce-pushed the feat/modules-pr/config branch from 1b3f795 to 8401dd6CompareMarch 6, 2021 10:54
@jcxldn
jcxldnforce-pushed the feat/modules-pr/config branch from 981453a to 32b638fCompareMarch 7, 2021 09:46
@jcxldn
jcxldn marked this pull request as ready for review March 7, 2021 17:42
ZachyFoxx
ZachyFoxx approved these changes Mar 12, 2021
View reviewed changes

@ZachyFoxxZachyFoxx left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

kaylendog
kaylendog requested changes Mar 13, 2021
View reviewed changes

@kaylendogkaylendog left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, minor changes :3

Comment threadbukkit/src/main/java/com/dumbdogdiner/stickyapi/bukkit/util/StartupUtil.java
Comment threadbungee/src/main/java/com/dumbdogdiner/stickyapi/bungeecord/util/StartupUtil.java
Comment threadcommon/src/main/java/com/dumbdogdiner/stickyapi/common/configuration/FileConfiguration.java
Comment thread...on/src/main/java/com/dumbdogdiner/stickyapi/common/configuration/providers/YamlProvider.java
Comment thread...on/src/main/java/com/dumbdogdiner/stickyapi/common/configuration/providers/YamlProvider.java Outdated
Comment thread...on/src/main/java/com/dumbdogdiner/stickyapi/common/configuration/providers/YamlProvider.java Outdated
Comment thread...on/src/main/java/com/dumbdogdiner/stickyapi/common/configuration/providers/YamlProvider.java Outdated
Comment thread...on/src/main/java/com/dumbdogdiner/stickyapi/common/configuration/providers/YamlProvider.java Outdated
Comment thread...rc/test/java/com/dumbdogdiner/stickyapi/common/configuration/providers/YamlProviderTest.java
import org.junit.jupiter.api.TestInstance.Lifecycle;

@TestInstance(Lifecycle.PER_CLASS)
public class YamlProviderTest {

@kaylendogkaylendogMar 13, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As mentioned above, a test that forces a concurrent read/write of the config to ensure that behaves as expected.

Comment threadcommon/src/main/java/com/dumbdogdiner/stickyapi/common/configuration/FileConfiguration.java
@jcxldn
jcxldn merged commit 5741c89 into feat/jcx/major/modules-defMar 17, 2021
@jcxldn
jcxldn deleted the feat/modules-pr/config branch March 21, 2021 13:42
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kaylendogkaylendogkaylendog requested changes
@kokumajikokumajikokumaji approved these changes
@ZachyFoxxZachyFoxxZachyFoxx approved these changes

Assignees

@jcxldnjcxldn

Labels

breakingdependenciesPull requests that update a dependency filep0 bugVery high priority bug (server crashing etc)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jcxldn@kaylendog@kokumaji@ZachyFoxx