Migrate all main/v5 code to OSS repo by stippi2 · Pull Request #83 · SAP/cloud-sdk-java

stippi2 · 2023-10-02T08:15:16Z

The process was:

Remove all module directories from OSS repo in one commit
Add all module directories from main/v5 branch (Oct 02 ~0900) to OSS repo in one commit authored by cloudsdk@sap.com
Update files in scripts with their version from scripts/python from main/v5
Look closely at the diff of all pom.xml files & restore changes from OSS Repo:
- Remove cx-server related stuff from archetypes resources
- modules-bom/pom.xml
  - Reformat
  - Restore <build> section with plugins from the previous OSS version of the file
- bom/pom.xml:
  - Reformat
  - Add <build> section with plugins (see above)
- Restore root POM Javadoc changes
- Restore root POM Jacoco changes (:warning: please check)
- Add property project.build.sourceEncoding in bom/pom.xml and modules-bom/pom.xml, required for Code Formatting check to pass. The OSS version of modules-bom/pom.xml previously configured the encoding property of the formatter plugin in the build section. This should be an equivalent change.
- Adjust the allowed "high" findings count for PMD, SpotBugs and FindBugs. (:warning: needs proper fix via correct exclusions)

⚠️ Please don't squash merge this one, in order to preserve authorship of cloudsdk@sap.com for all source code.

Not sure if this is the right place for the fix.

TODO: The actual fix is likely to correctly fill and propagate the value of `codeAnalysisExclusions` in pom.xml.

...ain/java/com/sap/cloud/sdk/cloudplatform/connectivity/ComplexDestinationPropertyFactory.java

...ivity/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultHttpDestination.java

+                        .warn(
+                            "Did not find any '{}' headers to add to the outgoing request, even though Authentication type '{}' is set.",
+                            HttpHeaders.AUTHORIZATION,
+                            AuthenticationType.TOKEN_FORWARDING);


.../java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationRetrievalStrategyResolver.java

+            .debug(
+                "Loading destination from reuse-destination-service with retrieval strategy {} and token exchange strategy {}.",
+                retrievalStrategy,
+                tokenExchangeStrategy);


CharlesDuboisSAP · 2023-10-02T12:22:41Z

~~The bom/pom.xml shouldn't have the extra plugins and the extra <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>.~~
~~The modules-bom/pom.xml has an extra <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> that can be removed.~~
~~(Minor) Every pom but the two boms are formatted with tabs, those two should be consistent~~
~~The use of http or https ://www.apache.org/licenses/LICENSE-2.0.txt is inconsistent throughout the repo~~
~~What is the new file .adr-dir.~~
Remove the commented jacoco config in the main parent pom
The PMD plugin does not find all 701 warnings, only 14
Findbugs found 800 bugs -> did it never work before? If yes then adjust thresholds
Discrepancy in the Checkstyle results, also adjusts thresholds after
~~Where can we find coverageThresholds? Also adjust them if the report can be found~~
~~line missing in intellij_java_style~~
Why was the config removed from the scp-cf-spring archetype?
~~Why was the Setup java step added to the .github/workflow.build.yml?~~
Remove the extra .gitignore files (our fault)
~~scripts/common/release_audience.py has an extra "Internal" audience, is this intentional?~~
~~Unintentional change line 40 in scripts/create_module_invetory_file.py~~

stippi2 · 2023-10-02T12:56:28Z

The bom/pom.xml shouldn't have the extra plugins and the extra <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>.

The modules-bom/pom.xml has an extra <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> that can be removed.

The "Code Formatting" job in the pipeline fails if the encoding property of the formatter-maven-plugin is not defined. Since those two POMs don't inherit from the sdk-parent POM, the property is not configured (it defaults to project.build.sourceEncoding). Maybe the POMs should be excluded from the formatting checks, but that doesn't seem plausible to me. Should the encoding be specified some other way?

CharlesDuboisSAP · 2023-10-02T13:34:55Z

scripts/common/release_audience.py has an extra "Internal" audience, is this intentional?

Yes, since at least one of the modules has the audience property with this value, and generating the modules_inventory.json file then fails. We do generate this faile as part of the pipeline and validate that it doesn't result in changes.

I will set the modules to Public and remove the audience type Internal.

CharlesDuboisSAP · 2023-10-02T13:41:07Z

.pipeline/config.yml

  codeCheck:
    checkstyle:
-      high: '3'
+      high: '14'
      normal: '-1'
      low: '-1'
    findbugs:
-      high: '0'
+      high: '10'
      normal: '-1'
      low: '-1'
    pmd:
-      high: '0'
+      high: '10'
      normal: '-1'
      low: '-1'


Where can we find coverageThresholds? Also adjust them if the report can be found

Can be found here and there is a TODO for adjusting it after migrating the code. Should I adjust within this PR?

Yes we should adjust the thresholds in this PR.
@newtork @KavithaSiva do you think we should investigate the difference in amount of findings?

The PMD plugin does not find all 701 warnings, only 14

Findbugs found 800 bugs -> did it never work before? If yes then adjust thresholds

Discrepancy in the Checkstyle results

Hi!
I don't know why I was added as a reviewer on this PR. I'm a technical writer working on Commerce Cloud. Could you please remove me? Thank you!

stippi2 · 2023-10-02T13:54:55Z

Could the PMD and CheckStyle differences be explained by the different rulesets (see root-POM changes)?

CharlesDuboisSAP · 2023-10-04T07:50:33Z

LGTM, but let's wait for someone else's review.

…-code

stippi2 · 2023-10-04T07:51:39Z

Let's get the excludes working for Jacoco coverage first...

newtork

yolo

bot-sdk-js and others added 15 commits October 2, 2023 09:35

Remove all code

a095730

temporarily strip .gitignore

00a9102

Add main/v5 code 202310020942 verbatim

456cd19

Copy over v5 release preparations

d845e19

Remove cx-server related resources from archetypes

3bbef77

Reformat bom/pom.xml

11993e8

Restore SpotBugs and PMD ruleset changes

e71479c

Restore JavaDoc properties changes

4b2ae93

Restore OSS root pom Jacoco changes

79e9cdd

Specify project.build.sourceEncoding

bf1d571

Not sure if this is the right place for the fix.

Update Python scripts with main/v5 version

365f38c

Force Java 17 for CodeQL

cf17e85

Increase allowed "high" findings

8341850

TODO: The actual fix is likely to correctly fill and propagate the value of `codeAnalysisExclusions` in pom.xml.

Fix POM formatting (sorting)

e83fdda

Fix POM indentation

de96f33

github-advanced-security bot found potential problems Oct 2, 2023

View reviewed changes

stippi2 added 2 commits October 2, 2023 11:42

Add build sections with plugins to bom/modules-bom

9118582

Cleanup, we already specify sourceEncoding

fb6e622

stippi2 marked this pull request as ready for review October 2, 2023 10:29

stippi2 requested review from CharlesDuboisSAP, KavithaSiva and adumontb October 2, 2023 10:30

stippi2 self-assigned this Oct 2, 2023

Remove invalid prepare-agent property

83fcaa8

stippi2 added 2 commits October 2, 2023 14:28

fully qualified jacoco plugin in mvn CLI

a04099a

Revert build plugins in modules-bom and bom

ff72a4e

stippi2 added 2 commits October 2, 2023 14:58

tabs vs. spaces

d769fc2

Use https:// consistently for license URL

c3c3d01

CharlesDuboisSAP reviewed Oct 2, 2023

View reviewed changes

KavithaSiva requested review from newtork and removed request for adumontb October 2, 2023 13:48

bot-sdk-js and others added 2 commits October 2, 2023 15:49

Set DwC modules to Public

bf0c5a8

Copy coverage thresholds

4c57b6c

stippi2 and others added 11 commits October 2, 2023 15:56

Fix module-inventory.json

11d92c0

Copy over PMD, SpotBugs and FindBugs config

e094b2b

Restore Jacoco, PMD, SpotBugs and CheckStyle config

6e42e69

Copy exclusions for PMD and CheckStyle

29c39b4

Bump allowed PMD warnings

cc31713

Reduced coverage

e780b67

Final coverage values

793537e

Removed dwc modules from internal profile

72d97e8

Fixed Javadoc merge

75cb402

Removed Jacoco comments and removed non-existing modules from scans

7976717

Sync coverage thresholds, exclude samples

be06e99

stippi2 and others added 2 commits October 4, 2023 09:50

Fix sample excludes

c1f51e7

Merge remote-tracking branch 'origin/migrate-v5-code' into migrate-v5…

6b87c6d

…-code

stippi2 added 5 commits October 4, 2023 10:03

Remove quote character from jacoco excludes

493a030

Remove quote character from Jacoco excludes

0a5f186

Restore removal of nonexisting modules

b59b238

Jacoco: Print # of classes with info per package

0a452e7

Reduce coverage thresholds

e7bf686

newtork approved these changes Oct 4, 2023

View reviewed changes

stippi2 merged commit 34c9fda into main Oct 4, 2023

stippi2 deleted the migrate-v5-code branch October 4, 2023 15:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate all main/v5 code to OSS repo#83

Migrate all main/v5 code to OSS repo#83
stippi2 merged 45 commits intomainfrom
migrate-v5-code

stippi2 commented Oct 2, 2023 •

edited

Loading

Uh oh!

Uh oh!

Check failure

Check failure

CharlesDuboisSAP commented Oct 2, 2023 •

edited by stippi2

Loading

Uh oh!

stippi2 commented Oct 2, 2023 •

edited

Loading

Uh oh!

CharlesDuboisSAP commented Oct 2, 2023

Uh oh!

CharlesDuboisSAP Oct 2, 2023 •

edited

Loading

Uh oh!

adumontb Oct 2, 2023

Uh oh!

stippi2 commented Oct 2, 2023

Uh oh!

CharlesDuboisSAP commented Oct 4, 2023

Uh oh!

stippi2 commented Oct 4, 2023

Uh oh!

newtork left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

stippi2 commented Oct 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Check failure

Check failure

CharlesDuboisSAP commented Oct 2, 2023 • edited by stippi2 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stippi2 commented Oct 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CharlesDuboisSAP commented Oct 2, 2023

Uh oh!

CharlesDuboisSAP Oct 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adumontb Oct 2, 2023

Choose a reason for hiding this comment

Uh oh!

stippi2 commented Oct 2, 2023

Uh oh!

CharlesDuboisSAP commented Oct 4, 2023

Uh oh!

stippi2 commented Oct 4, 2023

Uh oh!

newtork left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

stippi2 commented Oct 2, 2023 •

edited

Loading

CharlesDuboisSAP commented Oct 2, 2023 •

edited by stippi2

Loading

stippi2 commented Oct 2, 2023 •

edited

Loading

CharlesDuboisSAP Oct 2, 2023 •

edited

Loading