You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2020/02/03 21:55:09 UTC
[GitHub] [activemq-nms-amqp] Havret opened a new pull request #53:
AMQNET-634: Use 4 as default msg priority
Havret opened a new pull request #53: AMQNET-634: Use 4 as default msg priority
URL: https://github.com/apache/activemq-nms-amqp/pull/53
This closes https://issues.apache.org/jira/browse/AMQNET-634.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-nms-amqp] michaelandrepearce merged pull request #53:
AMQNET-634: Use 4 as default msg priority
Posted by GitBox <gi...@apache.org>.
michaelandrepearce merged pull request #53: AMQNET-634: Use 4 as default msg priority
URL: https://github.com/apache/activemq-nms-amqp/pull/53
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-nms-amqp] michaelpearce-gain commented on a change in
pull request #53: AMQNET-634: Use 4 as default msg priority
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #53: AMQNET-634: Use 4 as default msg priority
URL: https://github.com/apache/activemq-nms-amqp/pull/53#discussion_r374543790
##########
File path: test/Apache-NMS-AMQP-Test/NmsMessageProducerTest.cs
##########
@@ -77,7 +77,7 @@ public void TestGetDisableTimeStamp()
public void TestPriorityConfiguration()
{
IMessageProducer producer = session.CreateProducer(null);
- Assert.AreEqual(NMSConstants.defaultPriority, producer.Priority);
+ Assert.AreEqual(MsgPriority.BelowNormal, producer.Priority);
Review comment:
my point here is as per our discussion imagine the api values were changed to fix more fully in correcting the api.... would want this to flag broken immediately, where as relying on the api values ... it wouldnt
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-nms-amqp] HavretGC commented on a change in pull request
#53: AMQNET-634: Use 4 as default msg priority
Posted by GitBox <gi...@apache.org>.
HavretGC commented on a change in pull request #53: AMQNET-634: Use 4 as default msg priority
URL: https://github.com/apache/activemq-nms-amqp/pull/53#discussion_r374542832
##########
File path: test/Apache-NMS-AMQP-Test/NmsMessageProducerTest.cs
##########
@@ -77,7 +77,7 @@ public void TestGetDisableTimeStamp()
public void TestPriorityConfiguration()
{
IMessageProducer producer = session.CreateProducer(null);
- Assert.AreEqual(NMSConstants.defaultPriority, producer.Priority);
+ Assert.AreEqual(MsgPriority.BelowNormal, producer.Priority);
Review comment:
It won't change, I guess. Or at least it shouldn't. It's an enum, that basically represents all available message priorities. I could use int instead but that would require casting.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-nms-amqp] michaelpearce-gain commented on a change in
pull request #53: AMQNET-634: Use 4 as default msg priority
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #53: AMQNET-634: Use 4 as default msg priority
URL: https://github.com/apache/activemq-nms-amqp/pull/53#discussion_r374540801
##########
File path: test/Apache-NMS-AMQP-Test/NmsMessageProducerTest.cs
##########
@@ -77,7 +77,7 @@ public void TestGetDisableTimeStamp()
public void TestPriorityConfiguration()
{
IMessageProducer producer = session.CreateProducer(null);
- Assert.AreEqual(NMSConstants.defaultPriority, producer.Priority);
+ Assert.AreEqual(MsgPriority.BelowNormal, producer.Priority);
Review comment:
I would be explicit here with using int 4 (so we explicitly test the spec), rather than relying on a constant from outside the test pack, which it could change....
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-nms-amqp] michaelpearce-gain commented on a change in
pull request #53: AMQNET-634: Use 4 as default msg priority
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #53: AMQNET-634: Use 4 as default msg priority
URL: https://github.com/apache/activemq-nms-amqp/pull/53#discussion_r374548608
##########
File path: test/Apache-NMS-AMQP-Test/NmsMessageProducerTest.cs
##########
@@ -77,7 +77,7 @@ public void TestGetDisableTimeStamp()
public void TestPriorityConfiguration()
{
IMessageProducer producer = session.CreateProducer(null);
- Assert.AreEqual(NMSConstants.defaultPriority, producer.Priority);
+ Assert.AreEqual(MsgPriority.BelowNormal, producer.Priority);
Review comment:
fair enuf.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-nms-amqp] HavretGC commented on a change in pull request
#53: AMQNET-634: Use 4 as default msg priority
Posted by GitBox <gi...@apache.org>.
HavretGC commented on a change in pull request #53: AMQNET-634: Use 4 as default msg priority
URL: https://github.com/apache/activemq-nms-amqp/pull/53#discussion_r374547545
##########
File path: test/Apache-NMS-AMQP-Test/NmsMessageProducerTest.cs
##########
@@ -77,7 +77,7 @@ public void TestGetDisableTimeStamp()
public void TestPriorityConfiguration()
{
IMessageProducer producer = session.CreateProducer(null);
- Assert.AreEqual(NMSConstants.defaultPriority, producer.Priority);
+ Assert.AreEqual(MsgPriority.BelowNormal, producer.Priority);
Review comment:
This is covered with transport level assertion:
```
byte priority = 4;
testPeer.ExpectTransfer(m => Assert.AreEqual(priority, m.Header.Priority));
```
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services