Reduce scope of members

[macdude357]

• Reduce scope of members for issue Use minimum scope for members throughout the code base #214

Closes #214

What has been done to verify that this works as intended?

Code compiles and tests pass

Why is this the best possible solution? Were any other approaches considered?

Used IntelliJ's code inspection to find members that could be changed to a more restrictive scope.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

None

Does this change require updates to documentation? If so, please file an issue at https://github.com/opendatakit/docs/issues/new and include the link below.

No

[ggalmazor]

Thanks, @macdude357!

It looks like IntelliJ was too optimistic about some of the changes. CircleCI fails to compile the project now:

/home/circleci/work/src/org/opendatakit/briefcase/model/form/FormKey.java:66: error: formDefn has private access in BriefcaseFormDefinition
    XmlElement root = XmlElement.from(formDef.formDefn.xml);

Did you run the tests with ./gradlew clean test? If tests pass and the project fails to build in CircleCI we might have to check the CI configuration.

[macdude357]

Sorry about that. I fixed my compile issues but branch master had an existing compile error that I don't know how to fix since it requires a new arg and I'm not sure what to put there:

briefcase/model/form/FormMetadataCommandsTest.java:82: error: method orElseThrow in class Optional cannot be applied to given types;
FormMetadata actualFormMetadata = formMetadataPort.fetch(key).orElseThrow();

[ggalmazor]

You're right, @macdude357, sorry about that. It looks like I slipped some errors into master without noticing. I'm working on a v2.0 branch that sets the source language level to Java 11 and those errors you're getting are because the master branch has to be compiled at a language level of Java 8. I'll work on a solution asap so that you can rebase your branch and we can process this PR.

[ggalmazor]

Hi @macdude357! You should rebase master into your branch to solve the issues with mismatching java language level calls.

[codecov-io]

Codecov Report

Merging #825 into master will decrease coverage by 0.02%.
The diff coverage is 47.05%.

@@             Coverage Diff              @@
##             master     #825      +/-   ##
============================================
- Coverage     48.68%   48.65%   -0.03%     
- Complexity     1643     1645       +2     
============================================
  Files           192      192              
  Lines         10314    10324      +10     
  Branches        743      743              
============================================
+ Hits           5021     5023       +2     
- Misses         4935     4943       +8     
  Partials        358      358

Impacted Files	Coverage Δ	Complexity Δ
src/org/opendatakit/briefcase/export/Model.java	88.57% <ø> (ø)	55 <0> (ø)	⬇️
...it/aggregate/parser/BaseFormParserForJavaRosa.java	25.98% <ø> (+0.07%)	18 <0> (ø)	⬇️
...ndatakit/briefcase/model/BriefcasePreferences.java	41.55% <ø> (ø)	14 <0> (ø)	⬇️
...sed/transfer/sourcetarget/CentralServerDialog.java	0% <ø> (ø)	0 <0> (ø)	⬇️
...ase/model/UpdatedBriefcaseFormDefinitionEvent.java	75% <0%> (-25%)	1 <0> (ø)
src/org/opendatakit/briefcase/pull/PullEvent.java	40% <0%> (-10%)	1 <0> (ø)
...opendatakit/briefcase/reused/http/CommonsHttp.java	57.57% <0%> (-0.89%)	11 <0> (ø)
...opendatakit/briefcase/model/TerminationFuture.java	35.71% <0%> (ø)	2 <0> (ø)	⬇️
.../org/opendatakit/briefcase/model/form/FormKey.java	85.71% <100%> (ø)	15 <0> (ø)	⬇️
...takit/briefcase/model/BriefcaseFormDefinition.java	33.14% <100%> (+0.37%)	16 <1> (+1)	⬆️
... and 4 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 a2941e0...d5edb11. Read the comment docs.

[macdude357]

I'll update the PR to get the code coverage numbers back up.

[ggalmazor]

I'll update the PR to get the code coverage numbers back up.

I wouldn't worry about it.

[ggalmazor]

I'll tag this one to be released in v1.8 because this PR can't be tested without a full regression test, which we only do in minor releases (going from v1.17 to v1.18)

[ggalmazor]

So, we decided to ship this in v1.17.2.

@kkrawczyk123, when testing this one, please, try to go through the normal use cases to ensure this PR doesn't break basic functionality. One platform (Windows) and One server (Aggregate on Tomcat) would be enough.

[kkrawczyk123]

Testes with success!
None regression has been identified on Windows. (according to the comment above)

@opendatakit-bot unlabel "needs testing"
@opendatakit-bot label "behavior verified"

[getodk-bot]

ERROR: Label "behavior identified" does not exist and was thus not added to this pull request.

[kkrawczyk123]

@opendatakit-bot label "behavior verified"