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

Update Doxygen file blocks to remove copyright and license information #1283

Merged
merged 2 commits into from
Jan 23, 2018

Conversation

Patater
Copy link
Contributor

@Patater Patater commented Jan 19, 2018

This PR separates the copyright and license information from the Doxygen documentation blocks. The documentation was also updated to have file and brief blocks for all header files in include/mbedtls/.

Work done by @dgreen-arm

Testing done:

  • make apidoc
  • Open apidoc/index.html and browse around a bit
    • Verified that copyright and license info does not appear in the "More..."-linked Detailed Description section of files
    • Verified that copyright and license info did appear in the "More..."-linked Detailed Description section of files on the live website (https://tls.mbed.org/api/base64_8h.html#details)
  • Successfully ran ./tests/scripts/doxygen.sh
  • Successfully ran ./tests/scripts/check-doxy-blocks.sh
  • Successfully ran all.sh

Copy link
Contributor

@k-stachowiak k-stachowiak left a comment

Choose a reason for hiding this comment

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

I checked all the headers in the /include directory and I can confirm the Doxygen blocks are separated from the copyright notices.

I have noticed that this is not true for the doxygen/input files, but I'm not sure if that is even an issue and it reaches beyond the scope of this PR. So I'm just leaving this comment.

@gilles-peskine-arm
Copy link
Contributor

FYI it's important to check scripts/config.pl full && make apidoc. I'm doing this now.

On the other hand, scripts/config.pl full && make apidoc, which is what scripts/apidoc_full.sh does, is known to be incomplete. I'm not sure what that script is really for.

@gilles-peskine-arm
Copy link
Contributor

gilles-peskine-arm commented Jan 23, 2018

@k-stachowiak @Patater Indeed the files in doxygen/input need to be updated. To check:

grep Copyright apidoc/*.html | grep -v _source.html

It would be best to update the sample configurations in config as well for consistency. They aren't normally typeset, but someone could take one and decide to typeset the documentation.

Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm left a comment

Choose a reason for hiding this comment

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

Everything that has been done, looks good (removed copyright from Doxygen output, kept it in the source files, and no other changed).

I sampled the rendered HTML of scripts/config.pl full && make apidoc, and I also sampled the diff between the corresponding output before and after the patch.

However, as noted, this is incomplete. Please update the files in configs/ and in doxygen/input/ as well.

dgreen-arm and others added 2 commits January 23, 2018 15:44
Remove the copyright block from the Doxygen comments, to clean up the
detailed description in the generated Doxygen output. Also, add \file and
\brief tags to all headers in doxygen/input.
@Patater Patater force-pushed the dev/jaeden/doxygen-cleanup branch from 47789d1 to 25facdd Compare January 23, 2018 15:45
@Patater
Copy link
Contributor Author

Patater commented Jan 23, 2018

The files in configs/ and in doxygen/input/ have now been updated.

All previously mentioned testing performed by me has been redone, with the exception of running all.sh which hasn't finished yet.

Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm left a comment

Choose a reason for hiding this comment

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

It all looks good now.

@Patater
Copy link
Contributor Author

Patater commented Jan 23, 2018

all.sh has finished running, and it passed.

@Patater
Copy link
Contributor Author

Patater commented Jan 23, 2018

CI failure is FreeBSD timing test

15:53:03 
15:53:03   TIMING tests note: will take some time!
15:53:03   TIMING test #1 (set_alarm / get_timer): passed
15:53:03   TIMING test #2 (set/get_delay        ): failed at line 459
15:53:03  cycles=0 ratio=0 millisecs=1001 secs=1 hardfail=0 a=800 b=400
15:53:03  elapsed(hires)=2308 elapsed(ctx)=1307 status(ctx)=2
15:53:03   Executed 23 test suites
15:53:03 
15:53:03   [ 1 tests FAIL ]

@Patater Patater merged commit 25facdd into Mbed-TLS:development Jan 23, 2018
mpg added a commit to mpg/mbedtls that referenced this pull request Aug 29, 2024
…estricted-20240823

Merge 2.28 into -restricted
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.

None yet

5 participants