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.

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

pkcs11 support [12826] #296

Merged
merged 15 commits into from
Dec 22, 2021
Merged

pkcs11 support [12826] #296

merged 15 commits into from
Dec 22, 2021

Conversation

IkerLuengo
Copy link
Contributor

No description provided.

Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>
Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>
Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>
Copy link
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.

Also, when building the documentation the following warning is issued twice:

WARNING: Error in "code-block" directive:
maximum 1 argument(s) allowed, 2 supplied.

A couple of CI tests are also failing. Locally, the spelling test is complaining about:

fastdds/security/pkcs11_support/pkcs11_support.rst:41: (openSSL)  This adds a new PKCS#11 engine that can be used with openSSL.
fastdds/security/pkcs11_support/pkcs11_support.rst:46: (linux)  For example, on a linux machine with 

(It seems that Linux is also always with the first letter capitalized 😅 )

docs/fastdds/security/pkcs11_support/pkcs11_support.rst Outdated Show resolved Hide resolved
docs/installation/sources/sources_linux.rst Outdated Show resolved Hide resolved
docs/installation/sources/sources_windows.rst Outdated Show resolved Hide resolved
docs/installation/sources/sources_linux.rst Outdated Show resolved Hide resolved
Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>
Signed-off-by: Iker Luengo <ikerluengo@eprosima.com>
Copy link
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.

Changes LGTM but there are a couple of tests that are still failing:

  • doc8-test
/home/eprosima/Fast-DDS/fastdds-docs-ws/src/fastdds-docs/docs/installation/sources/sources_windows.rst:4: D000 Duplicate explicit target name: "repository".
/home/eprosima/Fast-DDS/fastdds-docs-ws/src/fastdds-docs/docs/installation/sources/sources_windows.rst:392: D005 No newline at end of file
  • docs.spell_check
fastdds/security/pkcs11_support/pkcs11_support.rst:42: (openSSL)  This adds a new PKCS#11 engine that can be used with openSSL.
installation/sources/sources_linux.rst:116: (Libp)  Libp11 and SoftHSM libraries
installation/sources/sources_linux.rst:118: (Libp)  Libp11 provides PKCS#11 support for OpenSSL. This is an optional dependency,
installation/sources/sources_windows.rst:153: (Libp)  Libp11 and SoftHSM libraries
installation/sources/sources_windows.rst:155: (Libp)  Libp11 provides PKCS#11 support for OpenSSL. This is an optional dependency,

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
Copy link
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 with green CI

JLBuenoLopez and others added 2 commits December 20, 2021 16:08
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>
Copy link
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 complaining. I have included the changes needed to pass it. The rest of the changes LGTM

docs/fastdds/security/pkcs11_support/pkcs11_support.rst Outdated Show resolved Hide resolved
docs/installation/sources/sources_linux.rst Outdated Show resolved Hide resolved
docs/installation/sources/sources_windows.rst Outdated Show resolved Hide resolved
docs/installation/sources/sources_windows.rst Outdated Show resolved Hide resolved
docs/installation/sources/sources_windows.rst Outdated Show resolved Hide resolved
docs/installation/sources/sources_linux.rst Outdated Show resolved Hide resolved
Copy link
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.

DCO should be fixed

@JLBuenoLopez
Copy link
Contributor

JLBuenoLopez commented Dec 22, 2021

CI run: http://ci.intranet.eprosima.com:8080/view/Fast%20DDS/job/fastdds_docs_manual/206/

Spell check has failed!

fastdds/api_reference/dds_pim/topic/topicdescription.rst:None: (ContentFilteredTopic)  eprosima::fastdds::dds::ContentFilteredTopic
fastdds/api_reference/dds_pim/domain/domainparticipant.rst:None: (filteredX)   named filteredX (and a corresponding 
fastdds/api_reference/dds_pim/domain/domainparticipant.rst:None: (myFilterFactory)  ), using a previously registered content filter factory, myFilterFactory. With only that, you will have filtering at the subscription side. If you also want to perform filtering in any application that publishes 
fastdds/api_reference/dds_pim/domain/domainparticipant.rst:None: (myFilterFactory)   X, then you also need to register the same definition of the ContentFilterFactory myFilterFactory in that application.
fastdds/api_reference/dds_pim/domain/domainparticipant.rst:None: (unregistration)  The unregistration of filter is not allowed if there are any existing 

MiguelBarro and others added 6 commits December 22, 2021 16:23
Co-authored-by: José Luis Bueno López <69244257+JLBuenoLopez-eProsima@users.noreply.github.com>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
Co-authored-by: José Luis Bueno López <69244257+JLBuenoLopez-eProsima@users.noreply.github.com>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
Co-authored-by: José Luis Bueno López <69244257+JLBuenoLopez-eProsima@users.noreply.github.com>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
Co-authored-by: José Luis Bueno López <69244257+JLBuenoLopez-eProsima@users.noreply.github.com>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
Co-authored-by: José Luis Bueno López <69244257+JLBuenoLopez-eProsima@users.noreply.github.com>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
Co-authored-by: José Luis Bueno López <69244257+JLBuenoLopez-eProsima@users.noreply.github.com>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez JLBuenoLopez force-pushed the feature/pkcs11-support branch from 646d521 to 53ed2d1 Compare December 22, 2021 15:23
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Copy link
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.

👍

@MiguelCompany
Copy link
Member

@MiguelCompany MiguelCompany merged commit eda66fc into master Dec 22, 2021
@MiguelCompany MiguelCompany deleted the feature/pkcs11-support branch December 22, 2021 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants