[GitHub] [wicket] andreikondratev commented on issue #347: Add more modal customizations
andreikondratev commented on issue #347: Add more modal customizations URL: https://github.com/apache/wicket/pull/347#issuecomment-469095284 Hi! Is this change got stuck? Should I do anything to proceed? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] bitstorm commented on issue #283: Wicket-6563 page store implementation
bitstorm commented on issue #283: Wicket-6563 page store implementation URL: https://github.com/apache/wicket/pull/283#issuecomment-469206921 Piiing! :-) This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on issue #347: Add more modal customizations
martin-g commented on issue #347: Add more modal customizations URL: https://github.com/apache/wicket/pull/347#issuecomment-469549206 I will create a ticket in JIRA for this PR and merge it. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g merged pull request #347: Add more modal customizations
martin-g merged pull request #347: Add more modal customizations URL: https://github.com/apache/wicket/pull/347 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on issue #347: Add more modal customizations
martin-g commented on issue #347: Add more modal customizations URL: https://github.com/apache/wicket/pull/347#issuecomment-469549730 https://issues.apache.org/jira/browse/WICKET-6640 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on issue #347: Add more modal customizations
martin-g commented on issue #347: Add more modal customizations URL: https://github.com/apache/wicket/pull/347#issuecomment-469550169 Thank you, @andruhon ! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] tiagostn opened a new pull request #348: WICKET-6642 Form.findSubmittingComponent always returns null
tiagostn opened a new pull request #348: WICKET-6642 Form.findSubmittingComponent always returns null URL: https://github.com/apache/wicket/pull/348 Form.findSubmittingComponent expects a request parameter with the wicket id of the submitting component. So I just add that to the SubmitLink. Same as in AjaxSubmitLink with AbstractDefaultAjaxBehavior atribute AjaxAttributeName.SUBMITTING_COMPONENT_NAME. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] andreikondratev commented on issue #347: Add more modal customizations
andreikondratev commented on issue #347: Add more modal customizations URL: https://github.com/apache/wicket/pull/347#issuecomment-470299287 Should I next time create a jira ticket for change like this? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] solomax commented on issue #347: Add more modal customizations
solomax commented on issue #347: Add more modal customizations URL: https://github.com/apache/wicket/pull/347#issuecomment-470342154 @andreikondratev JIRA tickets helps to create release notes So, I believe, it will speed up things This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on issue #347: Add more modal customizations
martin-g commented on issue #347: Add more modal customizations URL: https://github.com/apache/wicket/pull/347#issuecomment-470397486 Yes, most users don't read the git commit history to see what's new in a release. Wicket distributions (.tar.gz and .zip) contain RELEASE_NOTES file with the export from JIRA. GitHub releases (actually Git tags) are not official Apache releases. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] tiagostn closed pull request #348: WICKET-6642 Form.findSubmittingComponent always returns null
tiagostn closed pull request #348: WICKET-6642 Form.findSubmittingComponent always returns null URL: https://github.com/apache/wicket/pull/348 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] asfgit closed pull request #283: Wicket-6563 page store implementation
asfgit closed pull request #283: Wicket-6563 page store implementation URL: https://github.com/apache/wicket/pull/283 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] mattrpav opened a new pull request #349: [WICKET-6647] Upgrade asm to 7.1 in master
mattrpav opened a new pull request #349: [WICKET-6647] Upgrade asm to 7.1 in master URL: https://github.com/apache/wicket/pull/349 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] mattrpav opened a new pull request #350: [WICKET-6647] Upgrade to asm 7.1 in 8.x
mattrpav opened a new pull request #350: [WICKET-6647] Upgrade to asm 7.1 in 8.x URL: https://github.com/apache/wicket/pull/350 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] solomax commented on issue #349: [WICKET-6647] Upgrade asm to 7.1 in master
solomax commented on issue #349: [WICKET-6647] Upgrade asm to 7.1 in master URL: https://github.com/apache/wicket/pull/349#issuecomment-476743542 @mattrpav I got build error with this change: ``` WARNING] Dependency convergence error for org.ow2.asm:asm:7.0 paths to dependency are: +-org.apache.wicket:wicket-ioc:9.0.0-SNAPSHOT +-cglib:cglib:3.2.10 +-org.ow2.asm:asm:7.0 and +-org.apache.wicket:wicket-ioc:9.0.0-SNAPSHOT +-org.ow2.asm:asm-util:7.1 +-org.ow2.asm:asm:7.1 and +-org.apache.wicket:wicket-ioc:9.0.0-SNAPSHOT +-org.ow2.asm:asm-util:7.1 +-org.ow2.asm:asm-tree:7.1 +-org.ow2.asm:asm:7.1 icket-ioc: Some Enforcer rules have failed. Look above for specific messages explaining why the rule failed ``` I'm afraid some additional changes are required BTW can you close #350, I'll cherry-pick this one into wicket-8.x This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] mattrpav commented on issue #349: [WICKET-6647] Upgrade asm to 7.1 in master
mattrpav commented on issue #349: [WICKET-6647] Upgrade asm to 7.1 in master URL: https://github.com/apache/wicket/pull/349#issuecomment-476747443 @solomax apologies for the build warn. I created this PR to see if there was some CI/CD tied to Wicket's PR's. This is a simple fix to exclude asm requested by cglib. I need to upgrade my local JDK and will re-test. Thanks This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] mattrpav commented on issue #349: [WICKET-6647] Upgrade asm to 7.1 in master
mattrpav commented on issue #349: [WICKET-6647] Upgrade asm to 7.1 in master URL: https://github.com/apache/wicket/pull/349#issuecomment-476748019 @solomax what is the preferred workflow for patches? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] mattrpav edited a comment on issue #349: [WICKET-6647] Upgrade asm to 7.1 in master
mattrpav edited a comment on issue #349: [WICKET-6647] Upgrade asm to 7.1 in master URL: https://github.com/apache/wicket/pull/349#issuecomment-476748019 @solomax re the wicket-8.x patch-- what is the preferred workflow for patches? Do you merge PR's from here or other? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] solomax commented on issue #349: [WICKET-6647] Upgrade asm to 7.1 in master
solomax commented on issue #349: [WICKET-6647] Upgrade asm to 7.1 in master URL: https://github.com/apache/wicket/pull/349#issuecomment-476748823 @mattrpav PR with the link to JIRA is perfectly fine :) You did everything right :) This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] mattrpav commented on issue #349: [WICKET-6647] Upgrade asm to 7.1 in master
mattrpav commented on issue #349: [WICKET-6647] Upgrade asm to 7.1 in master URL: https://github.com/apache/wicket/pull/349#issuecomment-476750648 There is a cglib-nodep that wicket-ioc could depend on, and then inherit the asm dependency (which is used by other wicket modules). Q: This seems to be a driver's choice option here-- is there a strong preference to stay tightly coupled to cglib's transitive dependency on asm, or would the cglib-nodep approach be useful? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] solomax commented on issue #349: [WICKET-6647] Upgrade asm to 7.1 in master
solomax commented on issue #349: [WICKET-6647] Upgrade asm to 7.1 in master URL: https://github.com/apache/wicket/pull/349#issuecomment-476751902 I would say `cglib-nodep` is better option since we have `asm` as separate dependency ... @martin-g, @svenmeier, @bitstorm WDYT? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on issue #349: [WICKET-6647] Upgrade asm to 7.1 in master
martin-g commented on issue #349: [WICKET-6647] Upgrade asm to 7.1 in master URL: https://github.com/apache/wicket/pull/349#issuecomment-476773719 :+1: if everything still works ! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] solomax commented on issue #349: [WICKET-6647] Upgrade asm to 7.1 in master
solomax commented on issue #349: [WICKET-6647] Upgrade asm to 7.1 in master URL: https://github.com/apache/wicket/pull/349#issuecomment-477232378 @mattrpav can you check if `cglib-nodep+latest_asm` will work as expected and update your PR? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] mattrpav commented on issue #349: [WICKET-6647] Upgrade asm to 7.1 in master
mattrpav commented on issue #349: [WICKET-6647] Upgrade asm to 7.1 in master URL: https://github.com/apache/wicket/pull/349#issuecomment-477249580 Yep, will do This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] mattrpav commented on issue #349: [WICKET-6647] Upgrade asm to 7.1 in master
mattrpav commented on issue #349: [WICKET-6647] Upgrade asm to 7.1 in master URL: https://github.com/apache/wicket/pull/349#issuecomment-477420384 My local build is hanging on wicket-core with the test below: > [INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 s - in org.apache.wicket.pageStore.SerializingPageStoreTest [INFO] Running org.apache.wicket.pageStore.FilePageStoreTest Exception in thread "Wicket-AsyncPageStore-PageSavingThread" java.lang.NullPointerException at org.apache.wicket.pageStore.FilePageStore.setPageType(FilePageStore.java:352) at org.apache.wicket.pageStore.FilePageStore.writeFile(FilePageStore.java:223) at org.apache.wicket.pageStore.FilePageStore.addPersistedPage(FilePageStore.java:196) at org.apache.wicket.pageStore.AbstractPersistentPageStore.addPage(AbstractPersistentPageStore.java:128) at org.apache.wicket.pageStore.AsynchronousPageStore$PageAddingRunnable.run(AsynchronousPageStore.java:287) at java.base/java.lang.Thread.run(Thread.java:834) Environment - Oracle JDK 11.2 Mac OSX This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g merged pull request #349: [WICKET-6647] Upgrade asm to 7.1 in master
martin-g merged pull request #349: [WICKET-6647] Upgrade asm to 7.1 in master URL: https://github.com/apache/wicket/pull/349 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on issue #349: [WICKET-6647] Upgrade asm to 7.1 in master
martin-g commented on issue #349: [WICKET-6647] Upgrade asm to 7.1 in master URL: https://github.com/apache/wicket/pull/349#issuecomment-477524980 Everything looks good here! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on issue #350: [WICKET-6647] Upgrade to asm 7.1 in 8.x
martin-g commented on issue #350: [WICKET-6647] Upgrade to asm 7.1 in 8.x URL: https://github.com/apache/wicket/pull/350#issuecomment-477525601 The change will be cherry-picked from `master` branch. No need of this PR. Thank you, @mattrpav ! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g closed pull request #350: [WICKET-6647] Upgrade to asm 7.1 in 8.x
martin-g closed pull request #350: [WICKET-6647] Upgrade to asm 7.1 in 8.x URL: https://github.com/apache/wicket/pull/350 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] solomax opened a new pull request #351: [WICKET-6648] initiate and onBeforeDownload signatures are changed
solomax opened a new pull request #351: [WICKET-6648] initiate and onBeforeDownload signatures are changed URL: https://github.com/apache/wicket/pull/351 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on issue #351: [WICKET-6648] initiate and onBeforeDownload signatures are changed
martin-g commented on issue #351: [WICKET-6648] initiate and onBeforeDownload signatures are changed URL: https://github.com/apache/wicket/pull/351#issuecomment-479947955 How much effort is to add an example usage with WebSockets to wicket-examples > WebSockets demo ? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] solomax commented on issue #351: [WICKET-6648] initiate and onBeforeDownload signatures are changed
solomax commented on issue #351: [WICKET-6648] initiate and onBeforeDownload signatures are changed URL: https://github.com/apache/wicket/pull/351#issuecomment-479948236 I'll add, should be easy :) This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] solomax commented on issue #351: [WICKET-6648] initiate and onBeforeDownload signatures are changed
solomax commented on issue #351: [WICKET-6648] initiate and onBeforeDownload signatures are changed URL: https://github.com/apache/wicket/pull/351#issuecomment-479955654 @martin-g it seems `mvn jetty:run` is not working in `wicket-examples`, how are you running it for quick tests? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] solomax commented on issue #351: [WICKET-6648] initiate and onBeforeDownload signatures are changed
solomax commented on issue #351: [WICKET-6648] initiate and onBeforeDownload signatures are changed URL: https://github.com/apache/wicket/pull/351#issuecomment-479963931 `mvn jetty:run-exploded` seems to work :) This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on issue #351: [WICKET-6648] initiate and onBeforeDownload signatures are changed
martin-g commented on issue #351: [WICKET-6648] initiate and onBeforeDownload signatures are changed URL: https://github.com/apache/wicket/pull/351#issuecomment-480159091 I use `StartExamples#main()` in the IDE. It is much easier to debug and use HotSwap code replace (no need to restart, especially with DCEVM). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g opened a new pull request #352: WICKET-6649 Store the applicationName, sessionId and key in WebSocket…
martin-g opened a new pull request #352: WICKET-6649 Store the applicationName, sessionId and key in WebSocket… URL: https://github.com/apache/wicket/pull/352 … Text/BinaryMessage and in IWebSocketConnection Downport of 12b1fb227564c22f4887dea23610be7cb8c5d977 for Wicket 8.x. It manages to trick maven-clirr-plugin (binary compatibility checker) by using dummy values for Application, session id and IKey This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on a change in pull request #352: WICKET-6649 Store the applicationName, sessionId and key in WebSocket…
martin-g commented on a change in pull request #352: WICKET-6649 Store the
applicationName, sessionId and key in WebSocket…
URL: https://github.com/apache/wicket/pull/352#discussion_r272461990
##
File path:
wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/api/message/BinaryMessage.java
##
@@ -16,29 +16,57 @@
*/
package org.apache.wicket.protocol.ws.api.message;
+import org.apache.wicket.Application;
+import org.apache.wicket.Session;
+import org.apache.wicket.mock.MockApplication;
+import org.apache.wicket.protocol.ws.api.registry.IKey;
+
/**
* A {@link IWebSocketMessage message} with binary data
*
* @since 6.0
*/
-public class BinaryMessage implements IWebSocketMessage
+public class BinaryMessage extends AbstractClientMessage
{
private final byte[] data;
private final int offset;
private final int length;
/**
-* Constructor.
+* Not used by Wicket since 8.5.0!
*
* @param data
* the binary message from the client
* @param offset
* the offset to read from
* @param length
-* how much data to read
+* the length of the data to read
*/
+ @Deprecated
public BinaryMessage(byte[] data, int offset, int length)
{
+ this(new MockApplication(), "", new IKey() {}, data, offset,
length);
Review comment:
Is this acceptable ?
It is needed to be able to downport the improvement made in Wicket 9.x.
This constructor is used only by Wicket. I do not expect the user
application to have made use of it, but I cannot be fully certain.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [wicket] solomax commented on a change in pull request #352: WICKET-6649 Store the applicationName, sessionId and key in WebSocket…
solomax commented on a change in pull request #352: WICKET-6649 Store the
applicationName, sessionId and key in WebSocket…
URL: https://github.com/apache/wicket/pull/352#discussion_r272464222
##
File path:
wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/api/message/BinaryMessage.java
##
@@ -16,29 +16,57 @@
*/
package org.apache.wicket.protocol.ws.api.message;
+import org.apache.wicket.Application;
+import org.apache.wicket.Session;
+import org.apache.wicket.mock.MockApplication;
+import org.apache.wicket.protocol.ws.api.registry.IKey;
+
/**
* A {@link IWebSocketMessage message} with binary data
*
* @since 6.0
*/
-public class BinaryMessage implements IWebSocketMessage
+public class BinaryMessage extends AbstractClientMessage
{
private final byte[] data;
private final int offset;
private final int length;
/**
-* Constructor.
+* Not used by Wicket since 8.5.0!
*
* @param data
* the binary message from the client
* @param offset
* the offset to read from
* @param length
-* how much data to read
+* the length of the data to read
*/
+ @Deprecated
public BinaryMessage(byte[] data, int offset, int length)
{
+ this(new MockApplication(), "", new IKey() {}, data, offset,
length);
Review comment:
I see no issues here :)
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [wicket] reiern70 commented on a change in pull request #352: WICKET-6649 Store the applicationName, sessionId and key in WebSocket…
reiern70 commented on a change in pull request #352: WICKET-6649 Store the
applicationName, sessionId and key in WebSocket…
URL: https://github.com/apache/wicket/pull/352#discussion_r272477274
##
File path:
wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/api/message/TextMessage.java
##
@@ -16,19 +16,46 @@
*/
package org.apache.wicket.protocol.ws.api.message;
+import org.apache.wicket.Application;
+import org.apache.wicket.mock.MockApplication;
+import org.apache.wicket.protocol.ws.api.registry.IKey;
import org.apache.wicket.util.lang.Args;
/**
* A {@link IWebSocketMessage message} with text data
*
* @since 6.0
*/
-public class TextMessage implements IWebSocketMessage
+public class TextMessage extends AbstractClientMessage
{
private final CharSequence text;
- public TextMessage(final CharSequence text)
+ /**
+* Not used by Wicket since 8.5.0!
+*
+* @param text
+* the message sent from the client
+*/
+ @Deprecated
+ public TextMessage(CharSequence text)
{
+ this(new MockApplication(), "", new IKey() {}, text);
Review comment:
I guess you can't use Application.get() because can't be sure there is
thread local Application attached to thread? Would it make sense to do this
with some "static"
```
ty {
return Application.get()
} catch() {
return new MockApplication():
}
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [wicket] martin-g commented on a change in pull request #352: WICKET-6649 Store the applicationName, sessionId and key in WebSocket…
martin-g commented on a change in pull request #352: WICKET-6649 Store the
applicationName, sessionId and key in WebSocket…
URL: https://github.com/apache/wicket/pull/352#discussion_r272478938
##
File path:
wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/api/message/TextMessage.java
##
@@ -16,19 +16,46 @@
*/
package org.apache.wicket.protocol.ws.api.message;
+import org.apache.wicket.Application;
+import org.apache.wicket.mock.MockApplication;
+import org.apache.wicket.protocol.ws.api.registry.IKey;
import org.apache.wicket.util.lang.Args;
/**
* A {@link IWebSocketMessage message} with text data
*
* @since 6.0
*/
-public class TextMessage implements IWebSocketMessage
+public class TextMessage extends AbstractClientMessage
{
private final CharSequence text;
- public TextMessage(final CharSequence text)
+ /**
+* Not used by Wicket since 8.5.0!
+*
+* @param text
+* the message sent from the client
+*/
+ @Deprecated
+ public TextMessage(CharSequence text)
{
+ this(new MockApplication(), "", new IKey() {}, text);
Review comment:
Wicket uses the new constructors at
`org.apache.wicket.protocol.ws.api.AbstractWebSocketProcessor#onMessage()` and
here there is no Application ThreadLocal for sure. But let's assume that there
are Application and Session thread locals - the IKey would be still unknown,
and this makes all attempts useless. It would only work if the developer
overrides the getter methods of Text/BinaryMessage, but then it would be better
to migrate to the new constructors.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [wicket] reiern70 commented on a change in pull request #352: WICKET-6649 Store the applicationName, sessionId and key in WebSocket…
reiern70 commented on a change in pull request #352: WICKET-6649 Store the
applicationName, sessionId and key in WebSocket…
URL: https://github.com/apache/wicket/pull/352#discussion_r272485506
##
File path:
wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/api/message/TextMessage.java
##
@@ -16,19 +16,46 @@
*/
package org.apache.wicket.protocol.ws.api.message;
+import org.apache.wicket.Application;
+import org.apache.wicket.mock.MockApplication;
+import org.apache.wicket.protocol.ws.api.registry.IKey;
import org.apache.wicket.util.lang.Args;
/**
* A {@link IWebSocketMessage message} with text data
*
* @since 6.0
*/
-public class TextMessage implements IWebSocketMessage
+public class TextMessage extends AbstractClientMessage
{
private final CharSequence text;
- public TextMessage(final CharSequence text)
+ /**
+* Not used by Wicket since 8.5.0!
+*
+* @param text
+* the message sent from the client
+*/
+ @Deprecated
+ public TextMessage(CharSequence text)
{
+ this(new MockApplication(), "", new IKey() {}, text);
Review comment:
ok
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [wicket] solomax commented on issue #351: [WICKET-6648] initiate and onBeforeDownload signatures are changed
solomax commented on issue #351: [WICKET-6648] initiate and onBeforeDownload signatures are changed URL: https://github.com/apache/wicket/pull/351#issuecomment-480915022 Is it OK to merge this PR? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] solomax merged pull request #351: [WICKET-6648] initiate and onBeforeDownload signatures are changed
solomax merged pull request #351: [WICKET-6648] initiate and onBeforeDownload signatures are changed URL: https://github.com/apache/wicket/pull/351 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on issue #352: WICKET-6649 Store the applicationName, sessionId and key in WebSocket…
martin-g commented on issue #352: WICKET-6649 Store the applicationName, sessionId and key in WebSocket… URL: https://github.com/apache/wicket/pull/352#issuecomment-482016572 Any more opinions ? Should we merge this PR or you think it might break user applications ? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] solomax commented on issue #352: WICKET-6649 Store the applicationName, sessionId and key in WebSocket…
solomax commented on issue #352: WICKET-6649 Store the applicationName, sessionId and key in WebSocket… URL: https://github.com/apache/wicket/pull/352#issuecomment-482018642 Hello @martin-g, The original request was to simplify message sending to some "subset" of websocket connections Not sure how this code change will help ... This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on issue #352: WICKET-6649 Store the applicationName, sessionId and key in WebSocket…
martin-g commented on issue #352: WICKET-6649 Store the applicationName, sessionId and key in WebSocket… URL: https://github.com/apache/wicket/pull/352#issuecomment-482020393 I understood the request as: `make the Application, Session and IKey available in TextMessage, BinaryMessage and IWebSocketConnection, so the application can use them when deciding to whom to send response message`. @reiern70 did I understand you correctly ? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] reiern70 commented on issue #352: WICKET-6649 Store the applicationName, sessionId and key in WebSocket…
reiern70 commented on issue #352: WICKET-6649 Store the applicationName, sessionId and key in WebSocket… URL: https://github.com/apache/wicket/pull/352#issuecomment-482020794 @martin-g As requester of this feature if there is any risk of breaking existing application I would prefer to use some "workaround". Also the new version of our application is already migrated to Wicket 8. And probably Wicket 9 will be out before we get it into production... So, we will almost certainly be able to use correct implementation on Wicket 9. Thanks again! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] reiern70 commented on issue #352: WICKET-6649 Store the applicationName, sessionId and key in WebSocket…
reiern70 commented on issue #352: WICKET-6649 Store the applicationName, sessionId and key in WebSocket… URL: https://github.com/apache/wicket/pull/352#issuecomment-482020970 > I understood the request as: `make the Application, Session and IKey available in TextMessage, BinaryMessage and IWebSocketConnection, so the application can use them when deciding to whom to send response message`. > @reiern70 did I understand you correctly ? Yes you did. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g closed pull request #352: WICKET-6649 Store the applicationName, sessionId and key in WebSocket…
martin-g closed pull request #352: WICKET-6649 Store the applicationName, sessionId and key in WebSocket… URL: https://github.com/apache/wicket/pull/352 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on issue #352: WICKET-6649 Store the applicationName, sessionId and key in WebSocket…
martin-g commented on issue #352: WICKET-6649 Store the applicationName, sessionId and key in WebSocket… URL: https://github.com/apache/wicket/pull/352#issuecomment-482024698 > As requester of this feature if there is any risk of breaking existing application I would prefer to use some "workaround". Also the new version of our application is already migrated to Wicket 8. And probably Wicket 9 will be out before we get it into production... So, we will almost certainly be able to use correct implementation on Wicket 9. In that case I will close this PR! Thank you! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] reiern70 commented on issue #352: WICKET-6649 Store the applicationName, sessionId and key in WebSocket…
reiern70 commented on issue #352: WICKET-6649 Store the applicationName, sessionId and key in WebSocket… URL: https://github.com/apache/wicket/pull/352#issuecomment-482056433 > Hello @martin-g, > The original request was to simplify message sending to some "subset" of websocket connections > Not sure how this code change will help ... Hi, If IKey of message is same as IKey of connection (page that sent message), then I can get all connections and exclude the one that have same IKey of message. That was my use case. @martin-g Is this reasoning correct? If, so your changes suffice IMHO. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on issue #352: WICKET-6649 Store the applicationName, sessionId and key in WebSocket…
martin-g commented on issue #352: WICKET-6649 Store the applicationName, sessionId and key in WebSocket… URL: https://github.com/apache/wicket/pull/352#issuecomment-482084027 That's correct! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] svenmeier opened a new pull request #353: WICKET-6656 bean validation handle required annotations
svenmeier opened a new pull request #353: WICKET-6656 bean validation handle required annotations URL: https://github.com/apache/wicket/pull/353 Let BeanValidationContext decide which annotations make a component required. This way projects can choose to let beans-validation check those annotations instead, see BeanValidationApplication as example. To make this possible, PropertyValidator accepts nulls now. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] bd2019us opened a new pull request #354: ReplaceAll() to replace() for non-regex patterns
bd2019us opened a new pull request #354: ReplaceAll() to replace() for non-regex patterns URL: https://github.com/apache/wicket/pull/354 [WICKET-6657](https://issues.apache.org/jira/browse/WICKET-6657) Removed performance overhead cost associated with replaceAll() when a non-regex pattern is used This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on a change in pull request #354: ReplaceAll() to replace() for non-regex patterns
martin-g commented on a change in pull request #354: ReplaceAll() to replace()
for non-regex patterns
URL: https://github.com/apache/wicket/pull/354#discussion_r277147275
##
File path: wicket-util/src/main/java/org/apache/wicket/util/lang/Args.java
##
@@ -170,7 +170,7 @@ public static boolean isFalse(final boolean argument,
final String msg, final Ob
*/
static String format(String msg, final Object... params)
{
- msg = msg.replaceAll("\\{\\}", "%s");
+ msg = msg.replace("\\{\\}", "%s");
Review comment:
Is the behavior the same ?
I think we need to remove the `\\` now.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [wicket] martin-g commented on a change in pull request #354: ReplaceAll() to replace() for non-regex patterns
martin-g commented on a change in pull request #354: ReplaceAll() to replace()
for non-regex patterns
URL: https://github.com/apache/wicket/pull/354#discussion_r277147289
##
File path: wicket-util/src/main/java/org/apache/wicket/util/lang/Args.java
##
@@ -170,7 +170,7 @@ public static boolean isFalse(final boolean argument,
final String msg, final Ob
*/
static String format(String msg, final Object... params)
{
- msg = msg.replaceAll("\\{\\}", "%s");
+ msg = msg.replace("\\{\\}", "%s");
Review comment:
Adding tests if there are no such will be good!
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [wicket] asfgit closed pull request #353: WICKET-6656 bean validation handle required annotations
asfgit closed pull request #353: WICKET-6656 bean validation handle required annotations URL: https://github.com/apache/wicket/pull/353 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g closed pull request #354: ReplaceAll() to replace() for non-regex patterns
martin-g closed pull request #354: ReplaceAll() to replace() for non-regex patterns URL: https://github.com/apache/wicket/pull/354 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] d3ns0n opened a new pull request #355: WICKET-6660 do not trim PasswordTextField input
d3ns0n opened a new pull request #355: WICKET-6660 do not trim PasswordTextField input URL: https://github.com/apache/wicket/pull/355 override FormComponent.shouldTrimInput() in PasswordTextField to prevent passwords from being trimmed This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] bitstorm opened a new pull request #356: Wicket 6662
bitstorm opened a new pull request #356: Wicket 6662 URL: https://github.com/apache/wicket/pull/356 see discussion at https://issues.apache.org/jira/browse/WICKET-6662 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] mattrpav opened a new pull request #357: [WICKET-6661] Upgrade to jquery 3.4.1
mattrpav opened a new pull request #357: [WICKET-6661] Upgrade to jquery 3.4.1 URL: https://github.com/apache/wicket/pull/357 Upgrade to jquery 3.4.1 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g merged pull request #357: [WICKET-6661] Upgrade to jquery 3.4.1
martin-g merged pull request #357: [WICKET-6661] Upgrade to jquery 3.4.1 URL: https://github.com/apache/wicket/pull/357 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] svenmeier commented on issue #355: WICKET-6660 do not trim PasswordTextField input
svenmeier commented on issue #355: WICKET-6660 do not trim PasswordTextField input URL: https://github.com/apache/wicket/pull/355#issuecomment-488947502 @d3ns0n please close this pull request if you're fine with Wicket continuing to trim passwords - IMHO it's a matter of taste so I'd prefer to keep. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on issue #356: Wicket 6662
martin-g commented on issue #356: Wicket 6662 URL: https://github.com/apache/wicket/pull/356#issuecomment-488947913 I've glanced over several files and the changes look good! I will make a proper review in the coming days. But I'd prefer to keep the old classes as deprecated for Wicket 9.x and remove them in Wicket 10. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] d3ns0n closed pull request #355: WICKET-6660 do not trim PasswordTextField input
d3ns0n closed pull request #355: WICKET-6660 do not trim PasswordTextField input URL: https://github.com/apache/wicket/pull/355 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] bitstorm commented on issue #356: Wicket 6662
bitstorm commented on issue #356: Wicket 6662 URL: https://github.com/apache/wicket/pull/356#issuecomment-489436880 @martin-g normally I would do the same thing, but to me these are more classes for internal uses. I don't think many people use them directly with Wicket. Once we remove them from Wicket code base they are completely useless. But it's not a problem if we decide to keep them in Wicket 9.0. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] bitstorm edited a comment on issue #356: Wicket 6662
bitstorm edited a comment on issue #356: Wicket 6662 URL: https://github.com/apache/wicket/pull/356#issuecomment-489436880 @martin-g normally I would do the same thing, but to me these are more classes for internal use. I don't think many people use them directly with Wicket. Once we remove them from Wicket code base they are completely useless. But it's not a problem if we decide to keep them in Wicket 9.0. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on issue #356: Wicket 6662
martin-g commented on issue #356: Wicket 6662 URL: https://github.com/apache/wicket/pull/356#issuecomment-489462577 @bitstorm My reasoning is: If the classes are public then they have to be deprecated before being removed. There is no way to tell how many applications use any of those classes. There are no any drawbacks in keeping them as deprecated for 1 major release. The size of the jars will be slightly bigger - no one cares about such difference. We won't support those classes since they are deemed to be removed. We keep them around just for users' convenience. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] bitstorm commented on issue #356: Wicket 6662
bitstorm commented on issue #356: Wicket 6662 URL: https://github.com/apache/wicket/pull/356#issuecomment-489975813 @martin-g done! I've reintroduced and deprecated package org.apache.wicket.util.time This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on a change in pull request #356: Wicket 6662
martin-g commented on a change in pull request #356: Wicket 6662
URL: https://github.com/apache/wicket/pull/356#discussion_r281520703
##
File path:
wicket-core/src/test/java/org/apache/wicket/ajax/AjaxRequestHandlerTest.java
##
@@ -284,14 +283,14 @@ void ajaxRedirectSetsNoCachingHeaders()
tester.startPage(new Wicket3921());
tester.clickLink("updatePage");
- assertEquals(Time.START_OF_UNIX_TIME.toRfc1123TimestampString(),
+ assertEquals(Instants.toRFC7231Format(Instant.EPOCH),
Review comment:
Why change the RFC ?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [wicket] martin-g commented on a change in pull request #356: Wicket 6662
martin-g commented on a change in pull request #356: Wicket 6662 URL: https://github.com/apache/wicket/pull/356#discussion_r281519669 ## File path: wicket-core/src/main/java/org/apache/wicket/request/resource/DynamicImageResource.java ## @@ -19,13 +19,12 @@ import java.awt.image.BufferedImage; import java.io.ByteArrayOutputStream; import java.io.IOException; - +import java.sql.Time; Review comment: this import looks wrong and unused. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on a change in pull request #356: Wicket 6662
martin-g commented on a change in pull request #356: Wicket 6662
URL: https://github.com/apache/wicket/pull/356#discussion_r281527093
##
File path:
wicket-util/src/main/java/org/apache/wicket/util/string/StringValue.java
##
@@ -548,7 +552,14 @@ public final Double toDoubleObject() throws
StringValueConversionException
*/
public final Duration toDuration() throws StringValueConversionException
{
- return Duration.valueOf(text, locale);
+ try
+ {
+ return Duration.parse(text);
+ }
+ catch (Exception e)
+ {
+ throw new StringValueConversionException("xxx", e);
Review comment:
`xxx`
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [wicket] martin-g commented on a change in pull request #356: Wicket 6662
martin-g commented on a change in pull request #356: Wicket 6662
URL: https://github.com/apache/wicket/pull/356#discussion_r281520703
##
File path:
wicket-core/src/test/java/org/apache/wicket/ajax/AjaxRequestHandlerTest.java
##
@@ -284,14 +283,14 @@ void ajaxRedirectSetsNoCachingHeaders()
tester.startPage(new Wicket3921());
tester.clickLink("updatePage");
- assertEquals(Time.START_OF_UNIX_TIME.toRfc1123TimestampString(),
+ assertEquals(Instants.toRFC7231Format(Instant.EPOCH),
Review comment:
Why change the RFC ? Is the format still the same ?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [wicket] martin-g commented on a change in pull request #356: Wicket 6662
martin-g commented on a change in pull request #356: Wicket 6662
URL: https://github.com/apache/wicket/pull/356#discussion_r281521341
##
File path:
wicket-core/src/test/java/org/apache/wicket/page/PageAccessSynchronizerTest.java
##
@@ -98,13 +106,13 @@ public void run()
T2 t2 = new T2();
t2.setName("t2");
t1.start();
- Duration.milliseconds(100).sleep();
+ Thread.sleep(100);
Review comment:
nit: a more direct translation would be: `TimeUnit.MILLISECONDS.sleep(100)`
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [wicket] martin-g commented on a change in pull request #356: Wicket 6662
martin-g commented on a change in pull request #356: Wicket 6662 URL: https://github.com/apache/wicket/pull/356#discussion_r281522816 ## File path: wicket-jmx/src/main/java/org/apache/wicket/jmx/wrapper/RequestCycleSettings.java ## @@ -117,9 +117,9 @@ public void setResponseRequestEncoding(final String responseRequestEncoding) * @see org.apache.wicket.jmx.RequestCycleSettingsMBean#setTimeout(java.lang.String) */ @Override - public void setTimeout(final String timeout) + public void setTimeout(final Duration timeout) Review comment: I am not sure JMX will properly construct a Duration from the String input. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] bitstorm commented on a change in pull request #356: Wicket 6662
bitstorm commented on a change in pull request #356: Wicket 6662 URL: https://github.com/apache/wicket/pull/356#discussion_r281580379 ## File path: wicket-jmx/src/main/java/org/apache/wicket/jmx/wrapper/RequestCycleSettings.java ## @@ -117,9 +117,9 @@ public void setResponseRequestEncoding(final String responseRequestEncoding) * @see org.apache.wicket.jmx.RequestCycleSettingsMBean#setTimeout(java.lang.String) */ @Override - public void setTimeout(final String timeout) + public void setTimeout(final Duration timeout) Review comment: @martin-g I think you've lost me here :-). The old version of this method took in input a String and used a custom 'human-friendly' format to build a duration. For example something like '1.5 days' would have been converted into 36 hours. I've simply removed this textual format in order to use directly a standard Duration class. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on a change in pull request #356: Wicket 6662
martin-g commented on a change in pull request #356: Wicket 6662 URL: https://github.com/apache/wicket/pull/356#discussion_r281585885 ## File path: wicket-jmx/src/main/java/org/apache/wicket/jmx/wrapper/RequestCycleSettings.java ## @@ -117,9 +117,9 @@ public void setResponseRequestEncoding(final String responseRequestEncoding) * @see org.apache.wicket.jmx.RequestCycleSettingsMBean#setTimeout(java.lang.String) */ @Override - public void setTimeout(final String timeout) + public void setTimeout(final Duration timeout) Review comment: JMX works with a text-based protocol, like HTTP. E.g. a user opens a UI and enters the values for the exposed settings. If the type of the property is a String or a primitive then all is fine (e.g. JMX will just use `Integer.parseInt(input)`). But if the of the property is of some complex type then someone has to help JMX to convert the String input to the target type. I doubt there is a default converter for `java.time.Duration`. You can test it by starting a Wicket app and then connect to the JVM via JConsole or similar tool (e.g. wicketstuff-jmx-panel) and try to set a new value for `RequestCycleSettings#timeout` This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] bitstorm commented on a change in pull request #356: Wicket 6662
bitstorm commented on a change in pull request #356: Wicket 6662
URL: https://github.com/apache/wicket/pull/356#discussion_r281646311
##
File path:
wicket-core/src/test/java/org/apache/wicket/ajax/AjaxRequestHandlerTest.java
##
@@ -284,14 +283,14 @@ void ajaxRedirectSetsNoCachingHeaders()
tester.startPage(new Wicket3921());
tester.clickLink("updatePage");
- assertEquals(Time.START_OF_UNIX_TIME.toRfc1123TimestampString(),
+ assertEquals(Instants.toRFC7231Format(Instant.EPOCH),
Review comment:
It seems the right RFC for Date formats is RFC7231:
https://tools.ietf.org/html/rfc7231#section-7.1.1.1
But I admit to be a little confused, especially in respect of what RFC1123
actually says about Date formats.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [wicket] mattrpav opened a new pull request #358: [WICKET-6665] Upgrade various dependencies
mattrpav opened a new pull request #358: [WICKET-6665] Upgrade various dependencies URL: https://github.com/apache/wicket/pull/358 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] svenmeier opened a new pull request #359: Wicket js cleanup
svenmeier opened a new pull request #359: Wicket js cleanup URL: https://github.com/apache/wicket/pull/359 Before moving to a new technology I'd rather clean up the current JavaScript first. I've took the effort to separate all changes into separate commits, so anyone interested can assess the impact of that change more easily. I'm looking at http://youmightnotneedjquery.com/ now. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on a change in pull request #359: Wicket js cleanup
martin-g commented on a change in pull request #359: Wicket js cleanup
URL: https://github.com/apache/wicket/pull/359#discussion_r282362119
##
File path: wicket-core/src/test/js/dom.js
##
@@ -112,38 +112,6 @@ jQuery(document).ready(function() {
equal( el.style.display, '', "Wicket.DOM.show should set
.style.display to ''." );
});
- test("toggleClass() - single CSS class", function() {
Review comment:
why those tests are removed ?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [wicket] martin-g commented on a change in pull request #359: Wicket js cleanup
martin-g commented on a change in pull request #359: Wicket js cleanup URL: https://github.com/apache/wicket/pull/359#discussion_r282361940 ## File path: wicket-core/src/test/js/all.html ## @@ -181,8 +181,6 @@ - Review comment: why this is no more needed ? I think it is OK to keep this test for the `input` event This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on a change in pull request #359: Wicket js cleanup
martin-g commented on a change in pull request #359: Wicket js cleanup URL: https://github.com/apache/wicket/pull/359#discussion_r282362774 ## File path: wicket-examples/src/main/java/org/apache/wicket/examples/ajax/builtin/modal/ModalContent1Page.html ## @@ -20,9 +20,5 @@ Open another modal dialog - - An example of a component that uses header contributions - Review comment: how this is related to the JS cleanup ? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] andruhon commented on a change in pull request #359: Wicket js cleanup
andruhon commented on a change in pull request #359: Wicket js cleanup URL: https://github.com/apache/wicket/pull/359#discussion_r282364806 ## File path: wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/modal/res/modal.js ## @@ -1143,23 +1085,9 @@ e.style.zIndex = Wicket.Window.Mask.zIndex; Review comment: I believe that dodgy HTML in getMarkup at LL981-1041 can also be heavily simplified. I think it was done that complex to make it drop shadow made from images, which is unnecessary in 2019, because support for browsers without `box-shadow` has dropped a while ago. I can do this refactoring later to make it look nicer as well. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on a change in pull request #359: Wicket js cleanup
martin-g commented on a change in pull request #359: Wicket js cleanup URL: https://github.com/apache/wicket/pull/359#discussion_r282368339 ## File path: wicket-core/src/test/js/all.html ## @@ -181,8 +181,6 @@ - Review comment: Ignore this. It is better to remove some code. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on a change in pull request #359: Wicket js cleanup
martin-g commented on a change in pull request #359: Wicket js cleanup URL: https://github.com/apache/wicket/pull/359#discussion_r282368719 ## File path: wicket-examples/src/main/java/org/apache/wicket/examples/ajax/builtin/modal/ModalContent1Page.html ## @@ -20,9 +20,5 @@ Open another modal dialog - - An example of a component that uses header contributions - Review comment: I saw the commit message. I am OK to do this change in the examples. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] svenmeier commented on a change in pull request #359: Wicket js cleanup
svenmeier commented on a change in pull request #359: Wicket js cleanup
URL: https://github.com/apache/wicket/pull/359#discussion_r282450510
##
File path: wicket-core/src/test/js/dom.js
##
@@ -112,38 +112,6 @@ jQuery(document).ready(function() {
equal( el.style.display, '', "Wicket.DOM.show should set
.style.display to ''." );
});
- test("toggleClass() - single CSS class", function() {
Review comment:
Because I propose to remove that method.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [wicket] svenmeier commented on a change in pull request #359: Wicket js cleanup
svenmeier commented on a change in pull request #359: Wicket js cleanup URL: https://github.com/apache/wicket/pull/359#discussion_r282451295 ## File path: wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/modal/res/modal.js ## @@ -1143,23 +1085,9 @@ e.style.zIndex = Wicket.Window.Mask.zIndex; Review comment: modal.js is a beast on its own. IMHO we shouldn't put any effort in it, but deprecate the whole thing instead. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on a change in pull request #359: Wicket js cleanup
martin-g commented on a change in pull request #359: Wicket js cleanup
URL: https://github.com/apache/wicket/pull/359#discussion_r282453579
##
File path: wicket-core/src/test/js/dom.js
##
@@ -112,38 +112,6 @@ jQuery(document).ready(function() {
equal( el.style.display, '', "Wicket.DOM.show should set
.style.display to ''." );
});
- test("toggleClass() - single CSS class", function() {
Review comment:
Let's deprecate it first.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [wicket] andruhon commented on a change in pull request #359: Wicket js cleanup
andruhon commented on a change in pull request #359: Wicket js cleanup URL: https://github.com/apache/wicket/pull/359#discussion_r282454458 ## File path: wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/modal/res/modal.js ## @@ -1143,23 +1085,9 @@ e.style.zIndex = Wicket.Window.Mask.zIndex; Review comment: You're not going to deprecate the org.apache.wicket.extensions.ajax.markup.html.modal.ModalWindow itself? I think it's good to refactor it with some new JS or maybe to prepare some replacement with similar interface. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] svenmeier commented on a change in pull request #359: Wicket js cleanup
svenmeier commented on a change in pull request #359: Wicket js cleanup
URL: https://github.com/apache/wicket/pull/359#discussion_r282456466
##
File path: wicket-core/src/test/js/dom.js
##
@@ -112,38 +112,6 @@ jQuery(document).ready(function() {
equal( el.style.display, '', "Wicket.DOM.show should set
.style.display to ''." );
});
- test("toggleClass() - single CSS class", function() {
Review comment:
We can add @deprecated to those methods in Wicket-8.x, but I'm afraid nobody
reads JSDoc anyway. Or log an error instead?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [wicket] martin-g commented on a change in pull request #359: Wicket js cleanup
martin-g commented on a change in pull request #359: Wicket js cleanup
URL: https://github.com/apache/wicket/pull/359#discussion_r282457492
##
File path: wicket-core/src/test/js/dom.js
##
@@ -112,38 +112,6 @@ jQuery(document).ready(function() {
equal( el.style.display, '', "Wicket.DOM.show should set
.style.display to ''." );
});
- test("toggleClass() - single CSS class", function() {
Review comment:
Both `@deprecated` and a warning.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [wicket] martin-g commented on a change in pull request #359: Wicket js cleanup
martin-g commented on a change in pull request #359: Wicket js cleanup URL: https://github.com/apache/wicket/pull/359#discussion_r282458532 ## File path: wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/modal/res/modal.js ## @@ -1143,23 +1085,9 @@ e.style.zIndex = Wicket.Window.Mask.zIndex; Review comment: Lately there are no many new tickets for ModalWindow but there are several old ones in JIRA which no one cared to work on, me included. I'm +1 to deprecate it but keep it around. The deprecation is just to tell the users "We don't want to maintain it anymore". But I think we should keep it for the ones which use it without problems. Migrating to another modal (Bootstrap or anything) is an adventure on its own. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] svenmeier commented on a change in pull request #359: Wicket js cleanup
svenmeier commented on a change in pull request #359: Wicket js cleanup URL: https://github.com/apache/wicket/pull/359#discussion_r282461028 ## File path: wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/modal/res/modal.js ## @@ -1143,23 +1085,9 @@ e.style.zIndex = Wicket.Window.Mask.zIndex; Review comment: ModalWindow is broken beyond repair - not only its JS, but the Wicket internals are broken too, e.g. the need to wrap a Form around it. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] svenmeier commented on a change in pull request #359: Wicket js cleanup
svenmeier commented on a change in pull request #359: Wicket js cleanup URL: https://github.com/apache/wicket/pull/359#discussion_r282465153 ## File path: wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/modal/res/modal.js ## @@ -1143,23 +1085,9 @@ e.style.zIndex = Wicket.Window.Mask.zIndex; Review comment: +1 we'll keep it around until someone has better ideas. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] ivaynberg commented on a change in pull request #359: Wicket js cleanup
ivaynberg commented on a change in pull request #359: Wicket js cleanup URL: https://github.com/apache/wicket/pull/359#discussion_r282535350 ## File path: wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/modal/res/modal.js ## @@ -1143,23 +1085,9 @@ e.style.zIndex = Wicket.Window.Mask.zIndex; Review comment: We have written a replacement for wicket modal that is ADA compliant and more flexible. I can rip out our dependencies and commit it. It does use jquery...i guess that part will have to be rewritten if we are migrating away. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] andruhon commented on a change in pull request #359: Wicket js cleanup
andruhon commented on a change in pull request #359: Wicket js cleanup URL: https://github.com/apache/wicket/pull/359#discussion_r282687431 ## File path: wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/modal/res/modal.js ## @@ -1143,23 +1085,9 @@ e.style.zIndex = Wicket.Window.Mask.zIndex; Review comment: That's a common issue of wicket. I love wicket, it's brilliant, but most of wicket's JS/HTML/CSS looks like 2005, and it has a bad impact on framework decisions people make, because the first thing people check is demos and resulting HTML, unfortunately, looks like a complete rubbish. I really want to do my best to improve this. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] andreikondratev opened a new pull request #360: update Dockerfile to point to tomcat:8.5-jre11
andreikondratev opened a new pull request #360: update Dockerfile to point to tomcat:8.5-jre11 URL: https://github.com/apache/wicket/pull/360 because a new build target is 11 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] andruhon commented on a change in pull request #359: Wicket js cleanup
andruhon commented on a change in pull request #359: Wicket js cleanup URL: https://github.com/apache/wicket/pull/359#discussion_r282724954 ## File path: wicket-core/src/main/java/org/apache/wicket/ajax/res/js/wicket-ajax-jquery.js ## @@ -36,66 +36,28 @@ return; } - /** Review comment: By the way, just for my education. What's the purpose of the `/*global DOMParser: true, ActiveXObject: true, console: true */` in javascript files? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on a change in pull request #359: Wicket js cleanup
martin-g commented on a change in pull request #359: Wicket js cleanup URL: https://github.com/apache/wicket/pull/359#discussion_r282746367 ## File path: wicket-core/src/main/java/org/apache/wicket/ajax/res/js/wicket-ajax-jquery.js ## @@ -36,66 +36,28 @@ return; } - /** Review comment: Those are **local** hints for JSHint. The global ones are defined at https://github.com/apache/wicket/blob/886f11de02782feaddc3349e1d7ee599922516c3/testing/wicket-js-tests/Gruntfile.js#L71-L98 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
