Github user RagnarPaulson commented on the issue:
https://github.com/apache/activemq-nms-amqp/pull/2
Thanks Tim, All good feedback. I've cleaned up the comments and white
space. Added a new sample, and improved some of the 'toString' methods used in
debugging/development.
---
Github user dpauls commented on the issue:
https://github.com/apache/activemq-nms-amqp/pull/2
@tabish121 I appreciate the effort you've put into reviewing this. Just a
quick ping to see if there's anything else we can do to get this PR merged in.
If you just need to take another
Github user RagnarPaulson commented on the issue:
https://github.com/apache/activemq-nms-amqp/pull/2
Got it. I've just pushed up the latest version. It still passes all the
tests using ActiveMQ as a broker, so I believe i got all the namespace changes
correct.
---
Github user tabish121 commented on the issue:
https://github.com/apache/activemq-nms-amqp/pull/2
I would just make the changes and amend the commit and force push it to the
PR branch, you can also commit things locally and squash later if you prefer
that route. Basically until this
Github user RagnarPaulson commented on the issue:
https://github.com/apache/activemq-nms-amqp/pull/2
Thanks Tim, All good feedback.
What is the process now? Should I commit my changes as a new commit, or
quash it back into the original commit?
Ragnar
---
Github user RagnarPaulson commented on the issue:
https://github.com/apache/activemq-nms-amqp/pull/2
Thanks. That seemed to work better. Shows up as one commit now in the
pull request.
---
Github user tabish121 commented on the issue:
https://github.com/apache/activemq-nms-amqp/pull/2
If you can't work out the squash then another way to fix things is to back
out the commit using a "git reset --soft HEAD~X" where 'X' is the number of
commits and then create a new commit
Github user RagnarPaulson commented on the issue:
https://github.com/apache/activemq-nms-amqp/pull/2
I've tried to follow the quash and rebase instructions outlined
https://github.com/servo/servo/wiki/Beginner's-guide-to-rebasing-and-squashing
But it's not clear to me that
Github user tabish121 commented on the issue:
https://github.com/apache/activemq-nms-amqp/pull/2
It is generally best practice to squash commits into one on a PR to make
life easier for the reviews and to make the commit history simpler. We don't
really need to preserve a commit
Github user RagnarPaulson commented on the issue:
https://github.com/apache/activemq-nms-amqp/pull/2
Good catch. In my eagerness to compress several commits into one pull
request I lost the .gitignore and inadvertently committed all the
binaries/output. I've removed them all now.
Github user gemmellr commented on the issue:
https://github.com/apache/activemq-nms-amqp/pull/2
I cant really help on the code side as .NET isn't an area I play, but from
a skim over the file list for curiosity it seemed like there are various
non-code elements in this PR which
11 matches
Mail list logo
