Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
chore(docs): update documentation for coder_workspace_tags #15620
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
chore(docs): update documentation for coder_workspace_tags #15620
Changes from all commits
2ad6f66
9f3812f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Can we be sure that the error is explicit and inform the user on how to fix it?
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.
There are some UI improvements to be made, but here are the main messages:
tags
attribute ondata.coder_workspace_tags
: https://github.com/coder/coder/blob/main/provisioner/terraform/tfparse/tfparse_test.go#L85coder_workspace_tags
value: https://github.com/coder/coder/blob/main/provisioner/terraform/tfparse/tfparse_test.go#L290coder_workspace_tags
value: https://github.com/coder/coder/blob/main/provisioner/terraform/tfparse/tfparse_test.go#L324 (This one needs to be improved)coder_workspace_tags
value: https://github.com/coder/coder/blob/main/provisioner/terraform/tfparse/tfparse_test.go#L356I'd rather not over-index on documenting package-level errors in documentation, but I can document these if you think it's necessary?
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.
Not needed in docs but with UI enhancements but if we can parse them and show to the user that would do the work.
cc: @chrifro consider this in context of #15447.
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.
It should end up in the API response returned by the backend, so all the FE needs to do is display it to the user 👍
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 we should create an issue in coder/internal to track adding terraform errors to API response.
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 agree, but this might already be in the scope of #15447
Your call though!
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.
Yes ✅
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.
But the value of the
coder_parameter
can itself come from any otherlocal
,resource
, ordata
. Right?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.
No, the default must be specified in the
coder_parameter
data source. I'll make this more explicit.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.
Are we exposing an explicit error if this is not the case to inform the user on what they can do to fix it. Think about it in context of #15447 .
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.
See below comment, but TL;DR yes.