-
Notifications
You must be signed in to change notification settings - Fork 1
Bugfix/update dependencies #11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| <groupId>com.codedx</groupId> | ||
| <artifactId>codedx-bamboo-plugin</artifactId> | ||
| <version>1.0.0-SNAPSHOT</version> | ||
| <version>2.0.0</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New target version is debatable, bumped major version since this breaks compatibility with (much) older Bamboo versions.
| <!-- note: change to 9.0.0 for local development with `atlas-run` etc., prior versions have a broken UI --> | ||
| <bamboo.version>7.1.1</bamboo.version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally with 7.1.1, 8.2.6, 9.0.0, 9.2.5, 9.3.4
7.1.1 was chosen since it apparently includes a fix for some icu4j dependency issue, where icu4j would throw a fatal error on startup for any Java 8 subversion > 255
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to keep this at 7.1.1 for backward compatibility or something? Just wondering why it's not 9.0.0 in the repo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It didn't seem right to me to set a minimum version that was higher/more recent than the actual minimum version, which would lock out users with 7.1.1 < version < 9.0.0. We don't have a specific requested minimum version right now
|
|
||
| <!-- Base64 encoding that works in both JVM and Android --> | ||
| <!-- Note: Required by Swagger-generated API client impl. for Basic Auth, but basic auth is not used here --> | ||
| <dependency> | ||
| <groupId>com.brsanthu</groupId> | ||
| <artifactId>migbase64</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing this dependency would require updates in a bunch of different swagger-generated files; since it's unused, no reason to go through all that effort
| // This gets called automatically | ||
| public void setBandanaManager(BandanaManager bandanaManager) { | ||
| this.bandanaManager = bandanaManager; | ||
| public ServerConfigManager(BandanaManager bandanaManager) { | ||
| ServerConfigManager.bandanaManager = bandanaManager; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dependency injection changed from setter-based to ctor-based in Bamboo 7.0.0
| @Override | ||
| protected ThreeTenDateTimeDeserializerBase<T> withLeniency(Boolean leniency) { | ||
| // not relevant for us | ||
| return this; | ||
| } | ||
|
|
||
| @Override | ||
| protected ThreeTenDateTimeDeserializerBase<T> withShape(JsonFormat.Shape shape) { | ||
| // not relevant for us | ||
| return this; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(No issues in practice with just returning this, proper impl. would require a few extra annoying constructors)
| <pluginRepositories> | ||
| <pluginRepository> | ||
| <releases> | ||
| <enabled>true</enabled> | ||
| <checksumPolicy>warn</checksumPolicy> | ||
| </releases> | ||
| <snapshots> | ||
| <updatePolicy>never</updatePolicy> | ||
| <checksumPolicy>warn</checksumPolicy> | ||
| </snapshots> | ||
| <id>atlassian-public</id> | ||
| <url>https://maven.atlassian.com/repository/public</url> | ||
| </pluginRepository> | ||
| </pluginRepositories> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May have been necessary in older build process, doesn't seem to do anything with latest atlassian SDK
| <beans xmlns="http://www.springframework.org/schema/beans" | ||
| xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
| xmlns:atlassian-scanner="http://www.atlassian.com/schema/atlassian-scanner" | ||
| xmlns:atlassian-scanner="http://www.atlassian.com/schema/atlassian-scanner/2" | ||
| xsi:schemaLocation="http://www.springframework.org/schema/beans | ||
| http://www.springframework.org/schema/beans/spring-beans-2.5.xsd | ||
| http://www.springframework.org/schema/beans/spring-beans.xsd | ||
| http://www.atlassian.com/schema/atlassian-scanner | ||
| http://www.atlassian.com/schema/atlassian-scanner/atlassian-scanner.xsd"> | ||
| <atlassian-scanner:scan-indexes/> | ||
| </beans> | ||
| </beans> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Required change for scanner maven plugin
baffles
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really familiar with bamboo at all but no red flags jump out at me with this, just the one question. Feel free to resolve if that's a no-op.
Updates various dependencies to versions without CVEs, and includes some required logic changes necessary for the new dependencies/minimum Bamboo target.
CDX-3074