INT-2720: UriTemplate support for S-WS transports #769

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
2 participants
Member

artembilan commented Mar 19, 2013

Remove deprecation in the WS module
Polishing failed tests

JIRA: https://jira.springsource.org/browse/INT-2720
https://jira.springsource.org/browse/INT-2530

Special thanks to Arjen Poutsma, who reviewed the UriTemplate and introduced OpaqueUriComponents.

Member

artembilan commented Apr 5, 2013

Rebased and polished

INT-2720: Add a note to the Reference Manual
Polishing <authorgroup> to avoid extra whitespace before name
Member

artembilan commented Apr 5, 2013

Added commit about Reference Manual.
Note: <authorgroup> with new doc-plugin there is an extra whitespace before a name. I've fixed it as it is in the Spring Framework, but it is still there in the PDF, as it is in the SPR.
In addition logo.png in the PDF has very bad quality.
I know: it's not for current PR, but just keep it in mind.

- public void setReplyChannel(MessageChannel replyChannel) {
- this.setOutputChannel(replyChannel);
- }
-
@garyrussell

garyrussell Apr 9, 2013

Member

While I understand why you did this, it seems like an unnecessary breaking change - at the very least it needs to be added to the migration guide.

@artembilan

artembilan Apr 9, 2013

Member

Got it!
I think I'll mention in the migration guide all public API changes.
From other side: how about to revert deletion and mark as Deprecated?

@garyrussell

garyrussell Apr 9, 2013

Member

I am not sure it's even necessary to deprecate - it's just a convenience method, conveying a more appropriate property name. So, I would simply reinstate it (I can do that during the merge - this PR looks good to me, aside from my comment about replacing tabs with spaces in the what's new).

src/reference/docbook/whats-new.xml
+ For more information see <xref linkend="tcp-events"/>.
+ </para>
+ </section>
+ </section>
@garyrussell

garyrussell Apr 9, 2013

Member

Why did you change all those tabs to spaces?

@artembilan

artembilan Apr 9, 2013

Member

I've remembered how you was saing me in my first PRs to avoid tabs in docs, however if it is OK now with new doc-plugin I would appreciate it if you polish it during the merge.
Thanks

@garyrussell

garyrussell Apr 9, 2013

Member

I don't recall that conversation - maybe it was someone else. AFAIK tabs are ok; mixed tabs and spaces are not (and there were a few here). Anyway, I've changed them back to tabs.

@@ -67,30 +112,117 @@ public void checkUriVariables() {
assertTrue(WebServiceIOException.class.equals(causeType) // offline
@garyrussell

garyrussell Apr 9, 2013

Member

This test is failing for me - e is a WebServiceTransportException.

Also, the console is full of messages like this...

Error! A startup class specified in smack-config.xml could not be loaded: org.jivesoftware.smackx.ServiceDiscoveryManager
Error! A startup class specified in smack-config.xml could not be loaded: org.jivesoftware.smackx.XHTMLManager
Error! A startup class specified in smack-config.xml could not be loaded: org.jivesoftware.smackx.muc.MultiUserChat
Error! A startup class specified in smack-config.xml could not be loaded: org.jivesoftware.smackx.bytestreams.ibb.InBandBytestreamManager
...
@artembilan

artembilan Apr 9, 2013

Member

e is a WebServiceTransportException

It's something new, after some merging.

Error! A startup class specified in smack-config.xml

it comes from smack: I was thinking that it wasn't so important...
So, I'll look at it this evening and take care about mentioned concerns too.

@artembilan

artembilan Apr 9, 2013

Member

From other side http://springsource.org isn't available :-), as it is mentioned in the test context UriVariableTests-context.xml

@garyrussell

garyrussell Apr 9, 2013

Member

Haha - sorry about that - it's working now - we should fix that test so it doesn't require the internet - not necessary for this PR though.

If we can get rid of the smack noise, I'd prefer it (but don't spend too much time on it).

INT-2720: Pr comments & polishing
* Revert removed public API
* Revert tabs in What's new
* Change real URL to fake to allow for tests to work off-line
* Add smackx dependency to avoid error messages in logs
Member

artembilan commented Apr 9, 2013

Pushed

garyrussell added a commit that referenced this pull request Apr 10, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment