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

[CT-1809] Improve displayed message under "Arguments" section for argumentless macro #358

Closed
hvassard opened this issue Jan 12, 2023 · 3 comments · Fixed by #359
Closed
Labels
enhancement New feature or request good_first_issue Good for newcomers

Comments

@hvassard
Copy link

Describe the feature

When the arguments of a macro hasn't been documented, the following message is displayed under the Arguments section : "Details are not available for this macro"

image

This is 100% OK, but the same message is displayed when a macro doesn't have any argument. This is a bit confusing as it could be interpreted as a lack of documentation, whereas there's nothing to document.

It would be nice to have 2 different messages :

  • one when the arguments hasn't been documented (e.g. "Macro argument(s) hasn't been documented.")
  • one when the macro doesn't take any argument (e.g. "This macro doesn't have any argument.")

Describe alternatives you've considered

A clear and concise description of any alternative solutions or features you've considered.

Easy quick win :

  • Improve the existing message with something like "Details are not available for this macro. This may be due to the fact that this macro doesn't have any argument or that they haven't been documented yet." to warns that maybe it isn't a lack of docs.

This issue relates to this one on dbt-core project. @jtcohen6 recently commented this issue and thinks this quick win would happen here.

Additional context

Is this feature database-specific? Which database(s) is/are relevant? Please include any other relevant context here.

I encountered this trouble on a dbt x Redshift project, but I don't think there's any link with Redshift.

Who will this benefit?

What kind of use case will this feature be useful for? Please be specific and provide examples, this will help us prioritize properly.

This would be useful for everyone using macros and docs, but especially people who don't have access to the source code.
Also, that would avoid some bad impressions on the customer's side when a dev team has been hired to build a dbt project, as the customer may think they forgot some parts of the docs.

Are you interested in contributing this feature?

Let us know if you want to write some code, and how we can help.

I' like to help you, but I'm not really available for the moment :(

@hvassard hvassard added enhancement New feature or request triage labels Jan 12, 2023
@github-actions github-actions bot changed the title Improve displayed message under "Arguments" section for argumentless macro [CT-1809] Improve displayed message under "Arguments" section for argumentless macro Jan 12, 2023
@MartinGuindon
Copy link
Contributor

Adding a +1 to this!

@jtcohen6
Copy link
Contributor

Thanks for opening @hvassard (and +1'ing @MartinGuindon)!

I agree that this would be a very quick win:

  • Improve the existing message with something like "Details are not available for this macro. This may be due to the fact that this macro doesn't have any argument or that they haven't been documented yet." to warns that maybe it isn't a lack of docs.

Whereas this would be much, much trickier to implement (as discussed in the original dbt-core issue):

It would be nice to have 2 different messages :

  • one when the arguments hasn't been documented (e.g. "Macro argument(s) hasn't been documented.")
  • one when the macro doesn't take any argument (e.g. "This macro doesn't have any argument.")

Going to mark this as a good_first_issue with the former in mind!

@jtcohen6 jtcohen6 added good_first_issue Good for newcomers and removed triage labels Jan 12, 2023
@MartinGuindon
Copy link
Contributor

I wouldn't know how to do the latter, but if we're just updating the existing message, I can take care of that. I'll make the change and issue a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good_first_issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants