allow test with pekko 1.0.x snapshot #333
Conversation
pjfanning
commented
Oct 18, 2023
•
pjfanning
commented
- adjusts the tagNumber logic to treat build numbers without an RC or M tag as newer than ones that have them
- example
|
@pjfanning Do you want to get this over the finish line? I am happy with the change, just need want to change from I can also update the PR branch myself if you have time constraints. |
I've committed a change to call it |
|
@pjfanning Also need to add it to the matrix at https://github.com/apache/incubator-pekko-http/blob/17da94039e5b63d2be8f73cfac673e2a6f8517ca/.github/workflows/nightly.yml#L18 if you want the tests to actually run it. |
| @@ -114,11 +116,12 @@ object PekkoDependency { | |||
| ep.toInt, | |||
| maj.toInt, | |||
| min.toInt, | |||
| Option(tagNumber).map(_.toInt), | |||
| Option(tagNumber).map(_.toInt).getOrElse(Integer.MAX_VALUE), | |||
There was a problem hiding this comment.
Why was this change needed? Is that part of
adjusts the tagNumber logic to treat build numbers without an RC or M tag as newer than ones that have then?
Don't see how using Integer.MAX_VALUE is correct for what its doing (i.e. testing against latest found snapshot)
There was a problem hiding this comment.
yes - this is related to adjusts the tagNumber logic to treat build numbers without an RC or M tag as newer than ones that have them
There was a problem hiding this comment.
Okay, but how is Integer.MAX_VALUE going to provide a correct value here, or am I missing something? Is it just some sentinel value that will get ignored later (if so add a comment because logic/reasoning behind it is unclear).
There was a problem hiding this comment.
Oh nvm, its for ordering and not the actual value. @pjfanning Can you just add a comment explaining that its there to fix ordering and then ill approve PR.
There was a problem hiding this comment.
The values are used only in sorting.
-RC1-will eval as 1- no
-RCor-Mwill now eval as MAX-INT - MAX-INT > 1 so sorting will treat non-RC/non-milestone releases as newer
Old logic:
-RC1-will eval asSome(1)- no
-RCor-Mwill eval asNone None<Some(1)so sorting will treat non-RC/non-milestone releases as older
There was a problem hiding this comment.
Understood, just add a comment in the code quickly explaining this because on first glance its misleading.
|
I found something else, i.e. #333 (comment) . Also PR needs to be rebased/merge conflict against main since there are conflicts. |
|
@pjfanning Good news its working, i.e. you can check the latest run at https://github.com/apache/incubator-pekko-http/actions/runs/6687481949 and to further confirm if you check the job at https://github.com/apache/incubator-pekko-http/actions/runs/6687481949/job/18168181064 you can see its picking up the latest |
* allow test with pekko 1.0.x snapshot * Update PekkoDependency.scala * Update PekkoDependency.scala * rename config value to `1.0.x` * Update nightly.yml to include 1.0.x run * Update PekkoDependency.scala
