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

Revert of #1660 #1690

Closed
wants to merge 1 commit into from
Closed

Revert of #1660 #1690

wants to merge 1 commit into from

Conversation

Dimi1010
Copy link
Collaborator

This PR reverts the formatting changes made in #1660.

The reason for the revert is issues with #1659. Verbatim blocks have issues parsing with the /// syntax. And since the PRs are linked, decided it was better to revert to the /** structure than risk breaking the documentation.

This reverts commit 91d2058.

@Dimi1010 Dimi1010 marked this pull request as ready for review January 17, 2025 22:56
@Dimi1010 Dimi1010 requested a review from seladb as a code owner January 17, 2025 22:56
@tigercosmos
Copy link
Collaborator

Why don't you just revert the places that have issue?

@Dimi1010
Copy link
Collaborator Author

Dimi1010 commented Jan 18, 2025

Why don't you just revert the places that have issue?

@tigercosmos Uniformity mostly. The issue isn't in these PRs per se. The issue is in the Packet++ PR (#1659), that still isn't merged. Its an issue with doxygen getting confused with verbatim blocks with /// comments. The revert here is mostly so we aren't left with half the project in /// format and the other in /** format.

@tigercosmos
Copy link
Collaborator

IMO, I would like to have /// most of the places, and only use /** */ whenever there is @verbatim. In addition, only 5 files contain @verbatim, I think we can just treat them as special cases.

@Dimi1010
Copy link
Collaborator Author

IMO, I would like to have /// most of the places, and only use /** */ whenever there is @verbatim. In addition, only 5 files contain @verbatim, I think we can just treat them as special cases.

I am personally fine with either, tbf. As I mentioned in the linked comment, there are options.
I decided to do a revert PR due to the fact that so far we have tried to have the documentation uniform.

@seladb What do you say?

@egecetin
Copy link
Collaborator

@Dimi1010 Not followed the issues so it might be off topic, but did you saw this? Maybe it can fix the issues you encountered?

@Dimi1010
Copy link
Collaborator Author

@Dimi1010 Not followed the issues so it might be off topic, but did you saw this? Maybe it can fix the issues you encountered?

@egecetin huh... that might work actually.

@Dimi1010 Dimi1010 closed this Jan 19, 2025
@seladb
Copy link
Owner

seladb commented Jan 19, 2025

@Dimi1010 I'm sorry for the delayed response. I can see that replacing @verbatim with @code{.unparsed} solves the issue, so that's great news! 🎉

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.

4 participants