RATIS-2429. ratis-bom module fails to deploy due to missing distributionManagement configuration by OneSizeFitsQuorum · Pull Request #1371 · apache/ratis
|
|
||
| <properties> | ||
| <ratis.thirdparty.version>1.0.10</ratis.thirdparty.version> | ||
| <ratis.thirdparty.version>1.0.11</ratis.thirdparty.version> |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch, I missed following my own comment:
| <!-- Contains all shaded thirdparty dependencies; make sure to also update in ratis-bom/pom.xml --> | |
| <ratis.thirdparty.version>1.0.11</ratis.thirdparty.version> |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was just an accidental discovery :)
Comment on lines +25 to +36
| <distributionManagement> | ||
| <repository> | ||
| <id>apache.staging.https</id> | ||
| <name>Apache Release Distribution Repository</name> | ||
| <url>https://repository.apache.org/service/local/staging/deploy/maven2</url> | ||
| </repository> | ||
| <snapshotRepository> | ||
| <id>apache.snapshots.https</id> | ||
| <name>Apache Development Snapshot Repository</name> | ||
| <url>https://repository.apache.org/content/repositories/snapshots</url> | ||
| </snapshotRepository> | ||
| </distributionManagement> |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we copy from root POM both distributionManagement and the properties it refers to, as is?
| <distributionManagement> | |
| <repository> | |
| <id>${distMgmtStagingId}</id> | |
| <name>${distMgmtStagingName}</name> | |
| <url>${distMgmtStagingUrl}</url> | |
| </repository> | |
| <snapshotRepository> | |
| <id>${distMgmtSnapshotsId}</id> | |
| <name>${distMgmtSnapshotsName}</name> | |
| <url>${distMgmtSnapshotsUrl}</url> | |
| </snapshotRepository> | |
| </distributionManagement> |
| <distMgmtSnapshotsId>apache.snapshots.https</distMgmtSnapshotsId> | |
| <distMgmtSnapshotsName>Apache Development Snapshot Repository</distMgmtSnapshotsName> | |
| <distMgmtSnapshotsUrl>https://repository.apache.org/content/repositories/snapshots</distMgmtSnapshotsUrl> | |
| <distMgmtStagingId>apache.staging.https</distMgmtStagingId> | |
| <distMgmtStagingName>Apache Release Distribution Repository</distMgmtStagingName> | |
| <distMgmtStagingUrl>https://repository.apache.org/service/local/staging/deploy/maven2</distMgmtStagingUrl> |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, there are no properties in the ratis-bom, so I think the current style is consistent. I think it's both OK whether to change it or not.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My concern is that if someone builds the project with custom repo (-DdistMgmtStagingUrl=... or similar), ratis-bom might unexpectedly get deployed to Apache. Copying the properties would prevent it, custom value would be applied to ratis-bom lke all other modules.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make Sense! Fixed!