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

Allow rendering tags on methods without any docs #7952

Merged
merged 1 commit into from Jul 18, 2019

Conversation

Blacksmoke16
Copy link
Member

Fixes #7895.

  • Only skip generating docs if the method does not have any docs and doesn't have Deprecated annotation
  • Adds specs for generating summary/docs for a method
  • Fixes some type links on Int Deprecated messages, so that they are linkable in the API docs

Copy link
Member

@bcardiff bcardiff left a comment

Choose a reason for hiding this comment

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

Maybe #summary(context, string) could return nil when there is no summary to emit.
If the same is done with #doc(context, string), then the return if !doc && !obj.annotations(@program.deprecated_annotation checks can go away.

WDYT? (But is good for approving on my behalf)

src/compiler/crystal/tools/doc/generator.cr Show resolved Hide resolved
@Blacksmoke16
Copy link
Member Author

Blacksmoke16 commented Jul 4, 2019

@bcardiff Not sure I follow. Isn't that what it's doing already? If there are no docs nor annotations on the method, it'll return early. Otherwise if there is one or the other, there are docs to generate.

EDIT: I suppose we could redo some of the logic to skip the doc block specific stuff if it there are none, and just handle adding the deprecated flag. However I'm not sure if it would be worth it.

@bcardiff
Copy link
Member

bcardiff commented Jul 4, 2019

I meant that the new return if !doc && !obj.annotations(@program.deprecated_annotation) seems odd. If the summary and doc methods returned nil in those cases, then the check is not needed.

It's just an idea for (maybe) cleaner code.

But as I said. Is good for approving. If the refactor make sense to you feel free to iterate.

Thanks!

Copy link
Member

@sdogruyol sdogruyol 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 @Blacksmoke16 🙏

@bcardiff bcardiff added this to the 0.30.0 milestone Jul 17, 2019
@RX14 RX14 merged commit 8366cc7 into crystal-lang:master Jul 18, 2019
@Blacksmoke16 Blacksmoke16 deleted the deprecated-doc-tag branch July 18, 2019 11:52
dnamsons pushed a commit to dnamsons/crystal that referenced this pull request Jan 10, 2020
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.

Deprecated annotation not adding for methods that don't have any docs
4 participants