Skip to content

deliveryTag -> For manual ack#9471

Merged
davsclaus merged 5 commits into
apache:mainfrom
mayurvaid-redvest:main
Mar 13, 2023
Merged

deliveryTag -> For manual ack#9471
davsclaus merged 5 commits into
apache:mainfrom
mayurvaid-redvest:main

Conversation

@mayurvaid-redvest
Copy link
Copy Markdown
Contributor

@mayurvaid-redvest mayurvaid-redvest commented Mar 7, 2023

Description

Target

  • I checked that the commit is targeting the correct branch (note that Camel 3 uses camel-3.x, whereas Camel 4 uses the main branch)

Tracking

  • If this is a large change, bug fix, or code improvement, I checked there is a JIRA issue filed for the change (usually before you start working on it).

Apache Camel coding standards and style

  • I checked that each commit in the pull request has a meaningful subject line and body.
  • I formatted the code using mvn -Pformat,fastinstall install && mvn -Psourcecheck

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 7, 2023

🌟 Thank you for your contribution to the Apache Camel project! 🌟

⚠️ Please note that the changes on this PR may be tested automatically.

If necessary Apache Camel Committers may access logs and test results in the job summaries!

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 7, 2023

Components tested:

Total Tested Failed ❌ Passed ✅
1 1 1 0

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 7, 2023

Components tested:

Total Tested Failed ❌ Passed ✅
1 1 1 0

@davsclaus
Copy link
Copy Markdown
Contributor

davsclaus commented Mar 8, 2023

Are you able to update the docs (see src/main/docs) about this delivery tag and how to do a manual ack

Copy link
Copy Markdown
Contributor

@oscerd oscerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think deliveryTag String should be declared in the Constants class.

@davsclaus
Copy link
Copy Markdown
Contributor

I think deliveryTag String should be declared in the Constants class.

Yes

@mayurvaid-redvest
Copy link
Copy Markdown
Contributor Author

Are you able to update the docs (see src/main/docs) about this delivery tag and how to do a manual ack

Thanks for reviewing , Will add this documentation

@davsclaus
Copy link
Copy Markdown
Contributor

any update?

@mayurvaid-redvest
Copy link
Copy Markdown
Contributor Author

any update?

Updated the docs and added constant as well

@github-actions
Copy link
Copy Markdown
Contributor

Components tested:

Total Tested Failed ❌ Passed ✅
1 1 1 0

@davsclaus
Copy link
Copy Markdown
Contributor

I wonder if the delivery tag only make sense to have as a header if ack mode is manual? Are there situations where you need this information elsewhere ?

@mayurvaid-redvest
Copy link
Copy Markdown
Contributor Author

mayurvaid-redvest commented Mar 13, 2023

I wonder if the delivery tag only make sense to have as a header if ack mode is manual? Are there situations where you need this information elsewhere ?

Yes Even i am not aware of scenario where we would need this elsewhere , But having and header would anyways help and gives flexibility to opt for it or not , What are your thoughts

@mayurvaid-redvest
Copy link
Copy Markdown
Contributor Author

Thanks @davsclaus / @oscerd , Are we good to merge this

@github-actions
Copy link
Copy Markdown
Contributor

Components tested:

Total Tested Failed ❌ Passed ✅
1 1 0 1

@davsclaus davsclaus merged commit bfba7e6 into apache:main Mar 13, 2023
davsclaus pushed a commit that referenced this pull request Mar 14, 2023
* deliveryTag -> For manual ack

* Adding DELIVERY_TAG as constant

* Referring DELIVERY_TAG constants

* Adding readme docs for manual acknowledgement

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants