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

Documentation and code cleanup #193

Merged
merged 10 commits into from
Apr 17, 2023
Merged

Documentation and code cleanup #193

merged 10 commits into from
Apr 17, 2023

Conversation

edwardalee
Copy link
Contributor

@edwardalee edwardalee commented Apr 13, 2023

This is a starting point to clean up the APIs and their docs for user-facing APIs. It includes a number of code organization changes, including removing variables and functions from user scope. PR #1696 should be merged first, or else some tests will fail.

This first step focuses mainly on tag.h and tag.c. The generated doxygen docs are much better, now suitable for pointing users to them. This PR is already big enough that I think we should merge this incrementally. I.e., merge this now, then continue by picking the next part of the API, such as target.h, to work on.

@edwardalee edwardalee marked this pull request as ready for review April 15, 2023 06:19
Copy link
Contributor

@petervdonovan petervdonovan left a comment

Choose a reason for hiding this comment

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

Thank you for deleting the duplicated documentations and some of the unconventional uses of header files. It is nice to see the number of LOC go down a bit. I also agree that it is good to remove non-API things from tag.h; this was a significant missing part of #189.

Copy link
Member

@lhstrh lhstrh left a comment

Choose a reason for hiding this comment

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

These changes look good, and I'm a big fan of removing these long license notices at the beginning of files. I think we should stick to a single LICENSE.md that we can use in all of our repos, however.

@edwardalee edwardalee merged commit 12e6d1c into main Apr 17, 2023
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