Skip to content

Add sendPreviousResourceOnDelete to fhir-notification-configs#8361

Closed
JosieLi-Google wants to merge 7 commits into
GoogleCloudPlatform:mainfrom
JosieLi-Google:yifangli3
Closed

Add sendPreviousResourceOnDelete to fhir-notification-configs#8361
JosieLi-Google wants to merge 7 commits into
GoogleCloudPlatform:mainfrom
JosieLi-Google:yifangli3

Conversation

@JosieLi-Google

@JosieLi-Google JosieLi-Google commented Jul 14, 2023

Copy link
Copy Markdown
healthcare: added `send_previous_resource_on_delete`

@modular-magician

Copy link
Copy Markdown
Collaborator

Oops! It looks like you're using an unknown release-note type in your changelog entries:

  • REPLACEME

Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md.

@modular-magician

Copy link
Copy Markdown
Collaborator

Oops! It looks like you're using an unknown release-note type in your changelog entries:

  • REPLACEME

Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md.

@modular-magician modular-magician requested a review from roaks3 July 14, 2023 21:52
@modular-magician

Copy link
Copy Markdown
Collaborator

Hello! I am a robot who works on Magic Modules PRs.

I've detected that you're a community contributor. @roaks3, a repository maintainer, has been assigned to assist you and help review your changes.

❓ First time contributing? Click here for more details

Your assigned reviewer will help review your code by:

  • Ensuring it's backwards compatible, covers common error cases, etc.
  • Summarizing the change into a user-facing changelog note.
  • Passes tests, either our "VCR" suite, a set of presubmit tests, or with manual test runs.

You can help make sure that review is quick by running local tests and ensuring they're passing in between each push you make to your PR's branch. Also, try to leave a comment with each push you make, as pushes generally don't generate emails.

If your reviewer doesn't get back to you within a week after your most recent change, please feel free to leave a comment on the issue asking them to take a look! In the absence of a dedicated review dashboard most maintainers manage their pending reviews through email, and those will sometimes get lost in their inbox.


@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Jul 14, 2023

@roaks3 roaks3 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I found some typos, but otherwise seems fine after a first pass.

I'll run tests now, but also make sure you've gone through https://g3doc.corp.google.com/company/teams/terraform/users/resource-coverage/contribution-guide/index.md?cl=head to make sure you're in the appropriate groups to have the tests run without needing approval.

Lastly, it looks like you may need to update the PR description to get the release note formatted properly.

Comment thread mmv1/products/healthcare/FhirStore.yaml Outdated
Comment thread mmv1/products/healthcare/FhirStore.yaml Outdated
@modular-magician modular-magician removed the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Jul 17, 2023
@roaks3

roaks3 commented Jul 18, 2023

Copy link
Copy Markdown
Contributor

Build error is:

/usr/local/lib/ruby/3.1.0/psych.rb:455:in `parse': (<unknown>): found a tab character where an indentation space is expected while scanning a block scalar at line 284 column 24 (Psych::SyntaxError)

I'm assuming there is a tab somewhere in your yaml that is making something unhappy.

@modular-magician

Copy link
Copy Markdown
Collaborator

Oops! It looks like no changelog entry is attached to this PR. Please include a release note block in the PR body, as described in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md:

```release-note:TYPE
Release note
```

@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Aug 23, 2023
@modular-magician

Copy link
Copy Markdown
Collaborator

Oops! It looks like you're using an unknown release-note type in your changelog entries:

  • Fhir PubSub Enhancement

Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md.

@modular-magician modular-magician removed the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Aug 24, 2023
@roaks3

roaks3 commented Aug 24, 2023

Copy link
Copy Markdown
Contributor

Quick note while waiting for the tests: your PRODUCT in the release note would be healthcare

@modular-magician

Copy link
Copy Markdown
Collaborator

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

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 2 files changed, 67 insertions(+), 5 deletions(-))
Terraform Beta: Diff ( 3 files changed, 11 insertions(+), 11 deletions(-))

@JosieLi-Google

Copy link
Copy Markdown
Author

Hi @roaks3 , to resolve the Diff report, should I simple git rebase?

@modular-magician

Copy link
Copy Markdown
Collaborator

Tests analytics

Total tests: 2987
Passed tests 2689
Skipped tests: 296
Affected tests: 2

Action taken

Found 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccHealthcareFhirStore_healthcareFhirStoreNotificationConfigsExample|TestAccDataSourceGoogleServiceAccountIdToken_impersonation

Get to know how VCR tests work

@modular-magician

Copy link
Copy Markdown
Collaborator

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccDataSourceGoogleServiceAccountIdToken_impersonation[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$


$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccHealthcareFhirStore_healthcareFhirStoreNotificationConfigsExample[Error message] [Debug log]

$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$
View the build log or the debug log for each test

@roaks3

roaks3 commented Aug 24, 2023

Copy link
Copy Markdown
Contributor

@JosieLi-Google it looks like something strange has happened since your first commit (maybe the merges?). The first commit appears to add a field, but if I look at the file changes for the PR currently, I'm only seeing changes to the description (and some other small tweaks).

I would suggest cleaning up the branch with something like this:

git rebase -i fc007a168b8ad73d9d3b2f06f9441a8a40bf661d
<remove merge commit lines from the file before saving>

Hopefully that cleans up whatever happened with the merges, and you can try:

git fetch origin main
git rebase origin/main

Which would catch up the branch, with your handful of commits on top.

If that doesn't work, you of course have other options, like git reset + git cherry-pick, or just starting fresh and copy-and-pasting.

@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Aug 25, 2023
@JosieLi-Google

Copy link
Copy Markdown
Author

@JosieLi-Google it looks like something strange has happened since your first commit (maybe the merges?). The first commit appears to add a field, but if I look at the file changes for the PR currently, I'm only seeing changes to the description (and some other small tweaks).

I would suggest cleaning up the branch with something like this:

git rebase -i fc007a168b8ad73d9d3b2f06f9441a8a40bf661d
<remove merge commit lines from the file before saving>

Hopefully that cleans up whatever happened with the merges, and you can try:

git fetch origin main
git rebase origin/main

Which would catch up the branch, with your handful of commits on top.

If that doesn't work, you of course have other options, like git reset + git cherry-pick, or just starting fresh and copy-and-pasting.

Hi @roak3, I think my changes are already submitted in #8507. I will double check if there is anything still missing, if not I will close the PR. Let me know if there is any concern. Thank you!

@roaks3

roaks3 commented Aug 29, 2023

Copy link
Copy Markdown
Contributor

Ah ok, that explains the strange diff after the merges. Thanks!

@roaks3

roaks3 commented Nov 14, 2023

Copy link
Copy Markdown
Contributor

@JosieLi-Google gentle ping on this. Since it has been sitting a while, we may just want to close this PR and come back if there is anything missing.

@roaks3 roaks3 closed this Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-approval Pull requests that need reviewer's approval to run presubmit tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants