Skip to content
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

update ref to 1.0.0.final #267

Merged
merged 4 commits into from
Apr 29, 2015
Merged

update ref to 1.0.0.final #267

merged 4 commits into from
Apr 29, 2015

Conversation

smaldini
Copy link
Contributor

WIP for #223

@viktorklang
Copy link
Contributor

Looks good so far :)

@smaldini
Copy link
Contributor Author

Any feedback on the grammar, I think so far so good on this side but I feel a bit dodgy on this aspect to check the related box :)

@viktorklang
Copy link
Contributor

@smaldini I've done a pass, but I'd like to have more eyes on it.

@smaldini
Copy link
Contributor Author

maybe we can start the release of the binary and checks the impls too.

@smaldini
Copy link
Contributor Author

Right let's start the testing process, should I publish final on codehaus before merging @viktorklang ?

@viktorklang
Copy link
Contributor

@smaldini I'm not sure, assuming we already validated the integrity of the bits (perhaps do a RC5-final binary comparison before pushing?) it seems like we should be able to push it to maven central.
Wdyt?

@smaldini
Copy link
Contributor Author

@viktorklang I'm usually risk averse and I would just merge and push to maven.

@viktorklang
Copy link
Contributor

@smaldini Check the SHA of the RC5 jars and compare to the SHA of the final jars before pushing, and I'll sleep better :)

@smaldini
Copy link
Contributor Author

I've rarely seen 1.0.0 that works for everyone anyway, thats why there is a minor digit hey 💃
But I will test SHA

@smaldini
Copy link
Contributor Author

there's a bunch of TCK javadoc missing but well...

@viktorklang
Copy link
Contributor

@smaldini documentation improvement is never done :)

@smaldini
Copy link
Contributor Author

Strange , .asc and md5 are different from version to another.

@smaldini
Copy link
Contributor Author

which is strange since I'm only one commit away. looking at the content

@smaldini
Copy link
Contributor Author

right Manifest is regenerated

Export-Package: org.reactivestreams;version="1.0.0.RC5"
Bundle-SymbolicName: org.reactivestreams.reactive-streams
Bundle-Version: 1.0.0.RC5

@viktorklang
Copy link
Contributor

@smaldini Alright, if that's the only diff then let's go :)

@smaldini
Copy link
Contributor Author

It is on its way to maven central, joyful tear.

@viktorklang
Copy link
Contributor

Awesome, @reactive-streams/contributors should update to it so we can
announce shortly.

Cheers,

On 28 Apr 2015 18:00, "Stephane Maldini" notifications@github.com wrote:

It is on its way to maven central, joyful tear.


Reply to this email directly or view it on GitHub
#267 (comment)
.

@smaldini
Copy link
Contributor Author

@viktorklang
Copy link
Contributor

@jsuereth and @rkuhn suspect that the ".final" part of the version number
may trip up version conflict resolution.
Since the ".final" serves no purpose, I propose we pull the "1.0.0.final"
and republish it as "1.0.0" while we still can. Is that OK, @smaldini?

Cheers,

On 28 Apr 2015 18:14, "Stephane Maldini" notifications@github.com wrote:

http://repo1.maven.org/maven2/org/reactivestreams/reactive-streams/1.0.0.final/


Reply to this email directly or view it on GitHub
#267 (comment)
.

@ldaley
Copy link
Contributor

ldaley commented Apr 28, 2015 via email

@rkuhn
Copy link
Member

rkuhn commented Apr 29, 2015

@alkemist The problem would be that OSGi considers "1.0.0" < "1.0.0.RC5", how do we set a different version in the OSGi manifest, something like "1.0.0.final" is actually the right thing for OSGi.

@ldaley
Copy link
Contributor

ldaley commented Apr 29, 2015

Still +1.

@rkuhn
Copy link
Member

rkuhn commented Apr 29, 2015

background on why using "1.0.0.final" for Maven is problematic: http://mojo.codehaus.org/versions-maven-plugin/version-rules.html (in short: "1.0.0.final" is interpreted as "0.0.0-1.0.0.final" and effectively only string comparison is used)

@rkuhn
Copy link
Member

rkuhn commented Apr 29, 2015

@alkemist I take it that your +1 is for the "1.0.0" maven version. In sbt I would know how to fix the OSGi version, how do we do this in gradle?

@ldaley
Copy link
Contributor

ldaley commented Apr 29, 2015

FWIW, Ivy (therefore SBT) and Gradle understand that final > rc.

@benjchristensen
Copy link
Contributor

I also think we should release 1.0.0 and then if OSGi is a mess release 1.0.1 as a patch release.

@ldaley
Copy link
Contributor

ldaley commented Apr 29, 2015

@rkuhn what exactly do you want to do? Just use 1.0.0.final in the OSGi manifest?

@rkuhn
Copy link
Member

rkuhn commented Apr 29, 2015 via email

@viktorklang
Copy link
Contributor

Solving all issues sounds good to me

Cheers,

On 29 Apr 2015 09:31, "Roland Kuhn" notifications@github.com wrote:

Yes, that would solve all the issues AFAICS.


Reply to this email directly or view it on GitHub
#267 (comment)
.

@smaldini
Copy link
Contributor Author

Gradle OSGI plugin was already taking care of the bundle version but yeah making explicit is not harmful.

@rkuhn
Copy link
Member

rkuhn commented Apr 29, 2015

I tried it out with Viktor’s instructions locally and this was the resulting manifest:

rk:libs rkuhn$ cat META-INF/MANIFEST.MF 
Manifest-Version: 1.0
Bundle-Description: Reactive Streams API
Export-Package: org.reactivestreams;version="1.0.0",1.0.0.release
Bundle-SymbolicName: org.reactivestreams.reactive-streams
Bundle-Version: 1.0.0,1.0.0.release
Bundle-Name: reactive-streams
Bundle-ManifestVersion: 2
Bnd-LastModified: 1424093440000
Bundle-Vendor: Reactive Streams SIG
Bundle-DocURL: http://reactive-streams.org
Created-By: 1.8.0_31 (Oracle Corporation)
Tool: Bnd-2.1.0.20130426-122213

I tried to find information about what that version string means but that syntax is not mentioned anywhere in the freely available docs (I do not have an OSGi specification handy).

@smaldini
Copy link
Contributor Author

Yeah Grade OSGI plugin uses the version you specify :)

@ldaley
Copy link
Contributor

ldaley commented Apr 29, 2015

Ah, use instructionReplace instead of instruction.

@smaldini
Copy link
Contributor Author

we don't need that extra line

@smaldini
Copy link
Contributor Author

Is that correct enough for another try :) ?

Archive:  reactive-streams-1.0.0.jar
   creating: META-INF/
  inflating: META-INF/MANIFEST.MF    
   creating: org/
   creating: org/reactivestreams/
  inflating: org/reactivestreams/Processor.class  
  inflating: org/reactivestreams/Publisher.class  
  inflating: org/reactivestreams/Subscriber.class  
  inflating: org/reactivestreams/Subscription.class  

smaldini$ cat META-INF/MANIFEST.MF 

Manifest-Version: 1.0
Bundle-Description: Reactive Streams API
Export-Package: org.reactivestreams;version="1.0.0"
Bundle-SymbolicName: org.reactivestreams.reactive-streams
Bundle-Version: 1.0.0
Bundle-Name: reactive-streams
Bundle-ManifestVersion: 2
Bnd-LastModified: 1430236558000
Bundle-Vendor: Reactive Streams SIG
Bundle-DocURL: http://reactive-streams.org
Created-By: 1.8.0_20 (Oracle Corporation)
Tool: Bnd-2.1.0.20130426-122213

@viktorklang
Copy link
Contributor

@smaldini no, "Bundle-Version: 1.0.0" is wrong according to for OSGi we need something like "Bundle-Version: 1.0.0.release" (@rkuhn can confirm)

@smaldini
Copy link
Contributor Author

Who uses OSGI again ? sorry fixing that.

@smaldini
Copy link
Contributor Author

  inflating: META-INF/MANIFEST.MF    
  inflating: org/reactivestreams/Processor.class  
  inflating: org/reactivestreams/Publisher.class  
  inflating: org/reactivestreams/Subscriber.class  
  inflating: org/reactivestreams/Subscription.class  

wifi-5-214:libs smaldini$ cat META-INF/MANIFEST.MF 

Manifest-Version: 1.0
Bundle-Description: Reactive Streams API
Export-Package: org.reactivestreams;version="1.0.0.release"
Bundle-SymbolicName: org.reactivestreams.reactive-streams
Bundle-Version: 1.0.0.release
Bundle-Name: reactive-streams
Bundle-ManifestVersion: 2
Bnd-LastModified: 1430236558000
Bundle-Vendor: Reactive Streams SIG
Bundle-DocURL: http://reactive-streams.org
Created-By: 1.8.0_20 (Oracle Corporation)
Tool: Bnd-2.1.0.20130426-122213

@smaldini
Copy link
Contributor Author

now ?

@viktorklang
Copy link
Contributor

I'm not sure about: "Export-Package: org.reactivestreams;version="1.0.0.release""
@rkuhn?

For the record, let it be known, OSGi is an instrument for inflicting pain.

@smaldini
Copy link
Contributor Author

I'm going to make a slide about that springio.net tomorrow I think...

@viktorklang
Copy link
Contributor

@smaldini a slide about what? OSGi?

@smaldini
Copy link
Contributor Author

Yeah 👍 who cares about specs that are actually changing something in async processing without involving operating systems type containers.

@rkuhn
Copy link
Member

rkuhn commented Apr 29, 2015

@smaldini Your latest manifest is looking good!

@viktorklang
Copy link
Contributor

@rkhun Just to make sure, is "1.0.0.release" considered to be newer than "1.0.0.RC5" by OSGi? (note difference in upper/lower case)

@rkuhn
Copy link
Member

rkuhn commented Apr 29, 2015

Yes, it is, see https://osgi.org/javadoc/r4v43/core/org/osgi/framework/Version.html (it uses String.compareTo).

@viktorklang
Copy link
Contributor

@rkuhn Thanks!

Alright, so it looks like we have a solution!

let's switch to version 1.0.0 with OSGi version being 1.0.0.release in the repo
generate the jars
inspect the jars to make sure they are OK
publish 1.0.0
delete the 1.0.0.final tag from the reactive-streams-jvm repository

Sounds good?

@smaldini
Copy link
Contributor Author

The tag is not here yet since the PR was not merged. Right now it can then :)

Sent from my iPhone

On 29 Apr 2015, at 2:29 pm, Viktor Klang (√) notifications@github.com wrote:

@rkuhn Thanks!

Alright, so it looks like we have a solution!

let's switch to version 1.0.0 with OSGi version being 1.0.0.release in the repo
generate the jars
inspect the jars to make sure they are OK
publish 1.0.0
delete the 1.0.0.final tag from the reactive-streams-jvm repository

Sounds good?


Reply to this email directly or view it on GitHub.

@viktorklang
Copy link
Contributor

@rkuhn @smaldini @benjchristensen @alkemist and @reactive-streams/contributors in general.

I think we ought to merge this PR and publish the fixed 1.0.0 ASAP so we can get unstuck in the release process. @rkuhn, since you discovered the problem and found an appropriate fix, would you like to do the honors of cutting the release, or should our "release general" @smaldini do it?

@rkuhn
Copy link
Member

rkuhn commented Apr 29, 2015

Yes, I’ll start with the release now.

@viktorklang
Copy link
Contributor

Godspeed

Cheers,

On 29 Apr 2015 18:08, "Roland Kuhn" notifications@github.com wrote:

Yes, I’ll start with the release now.


Reply to this email directly or view it on GitHub
#267 (comment)
.

rkuhn added a commit that referenced this pull request Apr 29, 2015
@rkuhn rkuhn merged commit 9f6d8a6 into master Apr 29, 2015
@rkuhn rkuhn deleted the 1.0.0.final-wip branch April 29, 2015 16:09
@smaldini
Copy link
Contributor Author

Thanks @rkuhn! I was literally in flight from London Publisher to Barcelona Subscriber, fortunately flights are using backpressure too.

Sent from my iPhone

On 29 Apr 2015, at 5:22 pm, Viktor Klang (√) notifications@github.com wrote:

@rkuhn @smaldini @benjchristensen @alkemist and @reactive-streams/contributors in general.

I think we ought to merge this PR and publish the fixed 1.0.0 ASAP so we can get unstuck in the release process. @rkuhn, since you discovered the problem and found an appropriate fix, would you like to do the honors of cutting the release, or should our "release general" @smaldini do it?


Reply to this email directly or view it on GitHub.

@viktorklang
Copy link
Contributor

@smaldini There is back pressure for sure, the longer the flight the worse back pain...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants