-
Notifications
You must be signed in to change notification settings - Fork 16
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
Complete Documentation Pages #622
Conversation
e400faf
to
da48e91
Compare
include/mrdocs/Metadata/Info.hpp
Outdated
@@ -200,7 +132,7 @@ struct MRDOCS_VISIBLE | |||
class is a certain type. | |||
*/ | |||
template <InfoKind K> | |||
struct IsInfo : Info | |||
struct TypedInfo : Info |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer we don't change the name of this, or use Type
anywhere in InfoNodes.inc
. We already have TypeInfo
, so this makes it pretty confusing. Prefer Kind
for the names of the macro parameter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already have TypeInfo, so this makes it pretty confusing.
Mmm... Good thinking. I forgot about TypeInfo. That is confusing. You can also see why IsInfo
is confusing: it's not a trait, and it has nothing to do with what it does. Is there another name you could suggest?
-InfoType
- InfoCategory
- ClassifiedInfo
- SpecificInfo
- CategorizedInfo
- TypeSpecificInfo
- TypedMetadata
- InfoDiscriminator
- InfoVariant
- TypeDefinedInfo
- ...
Prefer Kind for the names of the macro parameter
I don't know what you mean here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alandefreitas IsInfoKind
? Regarding the name of the macro parameter, I'd use KIND
rather than TYPE
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the Is*
prefix makes it look like a trait, though. Maybe that's a convention somewhere else, but I've never seen an is*
class that's not a trait.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
InfoBase
? InfoCommon
? InfoCommonBase
? Anything that doesn't contain "type" :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I like InfoCommonBase for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
The PR completes the documentation to reflect the stage of this project properly.
This PR also includes several commits fixing smaller problems with CI, such as relevant information about errors not being output and wrong or insufficient error messages.
Unfortunately, GitHub updated the Windows runner while this PR was WIP, which broke CI for unrelated reasons:
This new image affects both this PR and unrelated PRs, but I've been trying to fix CI in passing so that we don't push new PRs when CI is broken. Among several commits fixing smaller problems with CI, the solution to the first problem was to re-upload LLVM Windows binaries to mrdocs.com and make the LLVM step fall back to these binaries until we can build LLVM in CI again.
However, I tried everything but couldn't fix the second problem. I can't even reproduce it locally and ran out of ideas. Any ideas are welcome here.