add a subcommand into diff processor to detect missing docs #11156

iyabchen · 2024-07-14T00:58:47Z

hashicorp/terraform-provider-google#14743
Part 1 - add missing docs command into diff-processor to detect new fields or new resource without doc. This only covers resource, NOT data sources.

For all field diffs, it gets the resource doc
Parse resource doc to get the list of existing arguments and attributes defined in the doc
- returned as a list of string, each string represented a flattend field, eg. a.b.c
- the parse might not be 100% accurate and not covered all corner cases. Whether a field is a nested object is determined by whether that field description contains a nested tag (some resource, eg. compute_instance params field does not have that in the description)
If the field is argument and not int he argument list returned by doc parser, then it's included in the result; similary for attribute
- however, i'm not sure about the argument/attribute condition written in the code is correct or not.

Release Note Template for Downstream PRs (will be copied)

modular-magician · 2024-07-14T01:08:19Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR hasn't generated any diffs, but I'll let you know if a future commit does.

github-actions · 2024-07-24T09:40:46Z

@shuyama1 This PR has been waiting for review for 3 weekdays. Please take a look! Use the label disable-review-reminders to disable these notifications.

iyabchen · 2024-07-25T19:09:03Z

Found some bugs already, need some rework.

modular-magician · 2024-08-28T22:56:29Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR hasn't generated any diffs, but I'll let you know if a future commit does.

tools/diff-processor/detector/detector.go

modular-magician · 2024-09-14T01:28:10Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR hasn't generated any diffs, but I'll let you know if a future commit does.

github-actions · 2024-09-17T09:41:45Z

@GoogleCloudPlatform/terraform-team @shuyama1 This PR has been waiting for review for 2 weeks. Please take a look! Use the label disable-review-reminders to disable these notifications.

modular-magician · 2024-09-24T19:42:15Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR hasn't generated any diffs, but I'll let you know if a future commit does.

github-actions · 2024-10-07T09:42:16Z

@iyabchen, this PR is waiting for action from you. If no action is taken, this PR will be closed in 28 days.

Please address any comments or change requests, or re-request review from a core reviewer if no action is required.

This notification can be disabled with the disable-automatic-closure label.

shuyama1 · 2024-10-07T17:13:39Z

Sorry for the delay! I just returned from vacation and am reviewing it now!

modular-magician · 2024-10-17T18:53:54Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR hasn't generated any diffs, but I'll let you know if a future commit does.

modular-magician · 2024-10-17T19:42:47Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR hasn't generated any diffs, but I'll let you know if a future commit does.

Errors

google provider:

Failed to build branch auto-pr-11156-old

shuyama1 · 2024-10-18T16:34:12Z

/gcbrun

modular-magician · 2024-10-18T16:43:34Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR hasn't generated any diffs, but I'll let you know if a future commit does.

github-actions · 2024-10-21T09:42:34Z

@iyabchen, this PR is waiting for action from you. If no action is taken, this PR will be closed in 14 days.

Please address any comments or change requests, or re-request review from a core reviewer if no action is required.

This notification can be disabled with the disable-automatic-closure label.

shuyama1 · 2024-10-21T16:36:36Z

@iyabchen, this PR is waiting for action from you. If no action is taken, this PR will be closed in 14 days.

Please address any comments or change requests, or re-request review from a core reviewer if no action is required.

This notification can be disabled with the disable-automatic-closure label.

I apologize for the delay in my review. I'm currently in another round of detailed review. Please ignore this message. Thank you for your patience!

shuyama1

Mostly looks good, some minor comments asking for more clarify and unit test coverages.

shuyama1 · 2024-10-25T20:11:23Z

tools/diff-processor/detector/detector.go

+
+type MissingDocField struct {
+	Field   string
+	Section string


I assume section indicates if this field is in argument or attribute section, IIRC? Can you add a small comment for it?

Section is removed since not differentiate argument and attribute now.

shuyama1 · 2024-10-25T20:25:39Z

tools/diff-processor/detector/detector.go

+	return ret, nil
+}
+
+func isAttribute(fieldDiff diff.FieldDiff) bool {


I might be reading this wrong - it seems that we still count the non-top level computed fields as attributes?

I’m starting to wonder that even if we only run the missing doc detector on newly added fields, inconsistencies in how attributes and arguments are divided in existing resource docs could still lead to issues(?), like for subfields added to existing objects. What are your thoughts on this?

Decided to not differentiate between argument and attribute, since finding some fields not conforming to that rule, eg. terraform_labels.

tools/diff-processor/cmd/detect_missing_docs_test.go

github-actions · 2024-10-31T09:42:21Z

@iyabchen, this PR is waiting for action from you. If no action is taken, this PR will be closed in 2 weekdays.

Please address any comments or change requests, or re-request review from a core reviewer if no action is required.

This notification can be disabled with the disable-automatic-closure label.

github-actions · 2024-11-04T09:42:29Z

@iyabchen, this PR is being closed due to inactivity.

modular-magician · 2024-11-04T17:19:18Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR hasn't generated any diffs, but I'll let you know if a future commit does.

modular-magician · 2024-11-05T02:59:50Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR hasn't generated any diffs, but I'll let you know if a future commit does.

modular-magician · 2024-11-05T20:06:47Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR hasn't generated any diffs, but I'll let you know if a future commit does.

shuyama1

LGTM overall! Thanks for putting in the work. Sorry for the delay in reviewing and merging—I’ve been a bit short on time to monitor CI changes. I’ll test it out this afternoon, and if everything goes well, I’ll merge it.

Next steps:

add the missing doc detector logic for the datasource (PR in progress)
surface this detector in the diff-report

iyabchen marked this pull request as ready for review July 19, 2024 23:23

iyabchen requested review from a team and shuyama1 and removed request for a team July 19, 2024 23:24

iyabchen marked this pull request as draft July 25, 2024 19:08

iyabchen removed the request for review from shuyama1 July 25, 2024 19:08

iyabchen force-pushed the diff-processor-missing-docs branch from a2f2de7 to 12ea5d1 Compare August 28, 2024 22:47

iyabchen commented Aug 28, 2024

View reviewed changes

tools/diff-processor/detector/detector.go Outdated Show resolved Hide resolved

shuyama1 self-requested a review September 4, 2024 01:28

iyabchen added 2 commits September 13, 2024 18:08

add a subcommand into diff processor to detect missing docs

3d2c9cc

a field is either attribute or argument not both

0e9ec45

iyabchen force-pushed the diff-processor-missing-docs branch from 12ea5d1 to 0e9ec45 Compare September 14, 2024 01:18

iyabchen marked this pull request as ready for review September 14, 2024 01:19

shuyama1 added the disable-review-reminders label Sep 23, 2024

remove outdated comment

1c355ed

add file path to the output

576606e

update output struct and fix test

162dab4

shuyama1 mentioned this pull request Oct 24, 2024

Detect missing docs for datasource #12043

Open

shuyama1 reviewed Oct 25, 2024

View reviewed changes

github-actions bot closed this Nov 4, 2024

shuyama1 reopened this Nov 4, 2024

shuyama1 added the disable-automatic-closure label Nov 4, 2024

github-actions bot requested a review from shuyama1 November 4, 2024 17:11

iyabchen added 2 commits November 4, 2024 17:49

Fix document parser to include more cases

eb0ee43

ignore section

ce3ffaa

iyabchen added 2 commits November 5, 2024 11:55

restore go.sum

e600906

fix go.sum and go.mod

5a23002

shuyama1 approved these changes Dec 5, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add a subcommand into diff processor to detect missing docs #11156

add a subcommand into diff processor to detect missing docs #11156

iyabchen commented Jul 14, 2024 •

edited

Loading

modular-magician commented Jul 14, 2024

github-actions bot commented Jul 24, 2024

iyabchen commented Jul 25, 2024

modular-magician commented Aug 28, 2024

modular-magician commented Sep 14, 2024

github-actions bot commented Sep 17, 2024

modular-magician commented Sep 24, 2024

github-actions bot commented Oct 7, 2024

shuyama1 commented Oct 7, 2024

modular-magician commented Oct 17, 2024

modular-magician commented Oct 17, 2024

shuyama1 commented Oct 18, 2024

modular-magician commented Oct 18, 2024

github-actions bot commented Oct 21, 2024

shuyama1 commented Oct 21, 2024

shuyama1 left a comment

shuyama1 Oct 25, 2024

iyabchen Nov 5, 2024

shuyama1 Oct 25, 2024

iyabchen Nov 5, 2024

github-actions bot commented Oct 31, 2024

github-actions bot commented Nov 4, 2024

modular-magician commented Nov 4, 2024

modular-magician commented Nov 5, 2024

modular-magician commented Nov 5, 2024

shuyama1 left a comment

add a subcommand into diff processor to detect missing docs #11156

Are you sure you want to change the base?

add a subcommand into diff processor to detect missing docs #11156

Conversation

iyabchen commented Jul 14, 2024 • edited Loading

modular-magician commented Jul 14, 2024

Diff report

github-actions bot commented Jul 24, 2024

iyabchen commented Jul 25, 2024

modular-magician commented Aug 28, 2024

Diff report

modular-magician commented Sep 14, 2024

Diff report

github-actions bot commented Sep 17, 2024

modular-magician commented Sep 24, 2024

Diff report

github-actions bot commented Oct 7, 2024

shuyama1 commented Oct 7, 2024

modular-magician commented Oct 17, 2024

Diff report

modular-magician commented Oct 17, 2024

Diff report

Errors

shuyama1 commented Oct 18, 2024

modular-magician commented Oct 18, 2024

Diff report

github-actions bot commented Oct 21, 2024

shuyama1 commented Oct 21, 2024

shuyama1 left a comment

Choose a reason for hiding this comment

shuyama1 Oct 25, 2024

Choose a reason for hiding this comment

iyabchen Nov 5, 2024

Choose a reason for hiding this comment

shuyama1 Oct 25, 2024

Choose a reason for hiding this comment

iyabchen Nov 5, 2024

Choose a reason for hiding this comment

github-actions bot commented Oct 31, 2024

github-actions bot commented Nov 4, 2024

modular-magician commented Nov 4, 2024

Diff report

modular-magician commented Nov 5, 2024

Diff report

modular-magician commented Nov 5, 2024

Diff report

shuyama1 left a comment

Choose a reason for hiding this comment

iyabchen commented Jul 14, 2024 •

edited

Loading