Skip to content

Documentation about Fast-CDR v2.0.0 and new annotations [19458]#550

Merged
JLBuenoLopez merged 12 commits intomasterfrom
feature/xcdr
Sep 27, 2023
Merged

Documentation about Fast-CDR v2.0.0 and new annotations [19458]#550
JLBuenoLopez merged 12 commits intomasterfrom
feature/xcdr

Conversation

@richiware
Copy link
Copy Markdown
Member

No description provided.

@richiware richiware changed the title Documentation about Fast-CDR v2.0.0 and new annotations Documentation about Fast-CDR v2.0.0 and new annotations [19458] Sep 6, 2023
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
Copy link
Copy Markdown
Contributor

@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 am missing the following information:

  • The usage section should include the new argument -de | --default_extensibility.
  • A note clarifying in Zero copy constraints stating that only FINAL types are considered to be plain (an a reference to the extensibility section).
  • New header files and constants should be included into the API reference:
    • eprosima::fastcdr::CdrVersion DEFAULT_XCDR_VERSION
    • DataRepresentationId_t DEFAULT_DATA_REPRESENTATION
  • Fast DDS Docs manual CI should be updated to use CMake 3.22. Probably the nightly job should also be updated.
  • Example CMakeList should use CMake minimum version 3.22.
  • The getting started application section explains the files generated by Fast DDS-Gen. It should be updated with the two new generated files.
  • The previous point also applies to the Python application

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Copy link
Copy Markdown
Member Author

I am missing the following information:

* [x]  The usage section should include the new argument `-de | --default_extensibility`.

* [x]  A note clarifying in [Zero copy constraints](https://fast-dds.docs.eprosima.com/en/latest/fastdds/use_cases/zero_copy/zero_copy.html#constraints) stating that only FINAL types are considered to be plain (an a reference to the extensibility section).

* [x]  New header files and constants should be included into the API reference:
  
  * eprosima::fastcdr::CdrVersion DEFAULT_XCDR_VERSION
  * DataRepresentationId_t DEFAULT_DATA_REPRESENTATION

* [x]  Fast DDS Docs manual CI should be updated to use CMake 3.22. Probably the nightly job should also be updated.

* [x]  Example CMakeList should use CMake minimum version 3.22.

* [x]  The [getting started application section](https://fast-dds.docs.eprosima.com/en/latest/fastdds/getting_started/simple_app/simple_app.html#build-the-topic-data-type) explains the files generated by Fast DDS-Gen. It should be updated with the two new generated files.

* [x]  The previous point also applies to the [Python application](https://fast-dds.docs.eprosima.com/en/latest/fastdds/getting_started/simple_python_app/simple_python_app.html#build-the-topic-data-type)

Done in 100644

Copy link
Copy Markdown
Contributor

@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.

CI is using fastrtps.repos from master and not the given branch. Once eProsima/Fast-DDS#3828 is merged we should run another CI

@JLBuenoLopez
Copy link
Copy Markdown
Contributor

The CI results can be found in this job

The following warnings are being displayed:

/home/runner/work/Fast-DDS/Fast-DDS/src/fastdds-docs/docs/../../src/fastdds-docs/docs/fastdds/getting_started/simple_app/includes/publisher.rst.rst:163: WARNING: line number spec is out of range(1-47): '47-48'
/home/runner/work/Fast-DDS/Fast-DDS/src/fastdds-docs/docs/../../src/fastdds-docs/docs/fastdds/getting_started/simple_app/includes/subscriber.rst.rst:118: WARNING: line number spec is out of range(1-47): '50-51'
../../src/fastdds-docs/docs/fastdds/getting_started/simple_app/includes/subscriber.rst:118: WARNING: Line spec '50-51': no lines pulled from include file '/home/runner/work/Fast-DDS/Fast-DDS/src/fastdds-docs/code/Examples/C++/DDSHelloWorld/CMakeLists.txt'

Also the doc8 tests complains about:

/home/runner/work/Fast-DDS/Fast-DDS/src/fastdds-docs/docs/fastdds/use_cases/zero_copy/zero_copy.rst:156: D001 Line too long

Finally, the spell check is also failing (though I think that it can be the same error as the doc8)

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
JLBuenoLopez
JLBuenoLopez previously approved these changes Sep 21, 2023
Copy link
Copy Markdown
Contributor

@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.

LGTM

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
Copy link
Copy Markdown
Contributor

@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.

LGTM

@JLBuenoLopez JLBuenoLopez merged commit 327dfa0 into master Sep 27, 2023
@JLBuenoLopez JLBuenoLopez deleted the feature/xcdr branch September 27, 2023 09:45
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.

2 participants