Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[11113] Pull mode property documentation #256

Merged
merged 14 commits into from
Apr 13, 2021
Merged

[11113] Pull mode property documentation #256

merged 14 commits into from
Apr 13, 2021

Conversation

JLBuenoLopez
Copy link
Contributor

This PR documents the pull mode property section.

Signed-off-by: JLBuenoLopez-eProsima joseluisbueno@eprosima.com

@richiprosima
Copy link
Contributor

Build status: Build Status

@JLBuenoLopez
Refs #11113: pull mode documentation (property)
d0def78 
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez JLBuenoLopez force-pushed the feature/push-mode-property branch from 8772557 to d0def78 Compare April 7, 2021 12:55
@JLBuenoLopez
Copy link
Contributor Author

Rebased so the CI is run using CMake.

@richiprosima
Copy link
Contributor

Buildstatus:BuildStatus

@richiprosima
Copy link
Contributor

Buildstatus:BuildStatus

@MiguelCompany
Refs 11113. Property renamed to correct value.
af0e665 
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
@MiguelCompany MiguelCompany force-pushed the feature/push-mode-property branch from 92b408a to af0e665 Compare April 8, 2021 05:49
@MiguelCompany
Refs 11113. Changed push mode explanation.
b05ef39 
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
@MiguelCompany
Refs 11113. Add note about intraprocess.
69a8cb5 
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
@richiprosima
Copy link
Contributor

Build status BuildStatus

@MiguelCompany MiguelCompany mentioned this pull request Apr 8, 2021
Merged
@richiprosima
Copy link
Contributor

Build status BuildStatus

@JLBuenoLopez
Copy link
Contributor Author

Please. fix CI:

  • WARNING: Title underline too short
``Push Mode`` QoS Policy
^^^^^^^^^^^^^^^^^^^^^^^
  • Spelling:
fastdds/property_policies/non_consolidated_qos.rst:19: (inmediately)  This implies they will send new samples to matched readers inmediately after adding them to their queue.

@MiguelCompany
Refs 11113. Fixed spelling and linters.
0162093 
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
@richiprosima
Copy link
Contributor

Build status BuildStatus

@richiprosima
Copy link
Contributor

Build status BuildStatus

JLBuenoLopez
JLBuenoLopez commented Apr 8, 2021
View reviewed changes
Copy link
Contributor Author

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

I cannot approve this PR as I have authored it.

LGTM though

docs/fastdds/property_policies/non_consolidated_qos.rst Outdated Show resolved Hide resolved
@MiguelCompany @JLBuenoLopez
Refs 11113. Apply suggestion
e89f7f2 
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

Co-authored-by: José Luis Bueno López <69244257+JLBuenoLopez-eProsima@users.noreply.github.com>
@MiguelCompany MiguelCompany requested review from EduPonz and removed request for MiguelCompany April 8, 2021 14:18
EduPonz
EduPonz suggested changes Apr 12, 2021
View reviewed changes
docs/fastdds/property_policies/non_consolidated_qos.rst Outdated Show resolved Hide resolved
docs/fastdds/property_policies/non_consolidated_qos.rst Outdated Show resolved Hide resolved
docs/fastdds/property_policies/non_consolidated_qos.rst Outdated Show resolved Hide resolved
docs/fastdds/property_policies/non_consolidated_qos.rst Outdated Show resolved Hide resolved
docs/fastdds/property_policies/non_consolidated_qos.rst Show resolved Hide resolved
docs/fastdds/property_policies/non_consolidated_qos.rst Show resolved Hide resolved
docs/fastdds/property_policies/non_consolidated_qos.rst Outdated Show resolved Hide resolved
docs/fastdds/property_policies/non_consolidated_qos.rst Outdated Show resolved Hide resolved
code/XMLTester.xml Show resolved Hide resolved
MiguelCompany and others added 7 commits April 13, 2021 09:52
@MiguelCompany @EduPonz
Refs 11113. Apply suggestions from code review
e334e60 
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

Co-authored-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
@MiguelCompany
Refs 11113. Use complete XML snippet.
0ae2c18 
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
@MiguelCompany
Refs 11113. Section name and cross reference.
21ac05c 
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
@MiguelCompany
Refs 11113. Rephrase push mode explanation.
e3a5137 
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
@MiguelCompany
Refs 11113. Reference tuning heartbeat section.
b31ded0 
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
@MiguelCompany
Refs 11113. Note on best-effort readers.
9f9622f 
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
@MiguelCompany
Refs 11113. Unordered list on warning.
07f2be0 
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
EduPonz
EduPonz previously approved these changes Apr 13, 2021
View reviewed changes
Copy link

@EduPonz EduPonz left a comment

Choose a reason for hiding this comment

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

LGTM

docs/fastdds/property_policies/non_consolidated_qos.rst Outdated Show resolved Hide resolved
@MiguelCompany
Refs 11113. Apply suggestion.
2535f2a 
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
@MiguelCompany MiguelCompany merged commit 3e9514b into master Apr 13, 2021
@MiguelCompany MiguelCompany deleted the feature/push-mode-property branch April 13, 2021 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MiguelCompany MiguelCompany MiguelCompany left review comments

@EduPonz EduPonz EduPonz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@JLBuenoLopez @richiprosima @EduPonz @MiguelCompany