Skip to content

fix: unbound variable _group in reset/dc-detect-version script (#1283)#1284

Merged
chadwhitacre merged 2 commits into
getsentry:masterfrom
lovetodream:master
Feb 15, 2022
Merged

fix: unbound variable _group in reset/dc-detect-version script (#1283)#1284
chadwhitacre merged 2 commits into
getsentry:masterfrom
lovetodream:master

Conversation

@lovetodream
Copy link
Copy Markdown
Contributor

@lovetodream lovetodream commented Jan 31, 2022

This would fix the reset script. [#1283ed.]

Because the dc-detect-version is only used in the reset script, I think it is fine to add the _group and _endgroup declarations directly.

@BYK BYK requested a review from chadwhitacre January 31, 2022 17:05
@chadwhitacre chadwhitacre mentioned this pull request Jan 31, 2022
@chadwhitacre
Copy link
Copy Markdown
Member

ref #1283

@chadwhitacre
Copy link
Copy Markdown
Member

Blech #1273. 😖

@lovetodream
Copy link
Copy Markdown
Contributor Author

Blech #1273. 😖

I Just noticed that #1273 was closed. Does this mean that this can be merged?

@chadwhitacre
Copy link
Copy Markdown
Member

Does this mean that this can be merged?

Almost. #1273 turned into #1289, which (it turns out) basically dupes #1290, which revives #1251, which we had to revert in #1272 because it effed up CI over in sentry. Kind of a tangle right now but I think we are making progress. 😁

Hopefully we can get back here soon ... 🤞

@chadwhitacre
Copy link
Copy Markdown
Member

@chadwhitacre
Copy link
Copy Markdown
Member

Still getting a garbled relay config.

lol sorry wrong PR! I thought the failure here was on #1289 😂 😭 Once we get that cleaned up and merged and rebase here we should be good ...

@aminvakil
Copy link
Copy Markdown
Collaborator

As #1315 has been merged, should this fix also reflect GCB too?

@chadwhitacre
Copy link
Copy Markdown
Member

Yeah, we need to rebase/update.

Comment thread install/dc-detect-version.sh
@aminvakil
Copy link
Copy Markdown
Collaborator

@lovetodream I guess this can be merged if you rebase from master and run the tests again.

Thanks for fixing this!

@chadwhitacre
Copy link
Copy Markdown
Member

/gcbrun

Copy link
Copy Markdown
Collaborator

@aminvakil aminvakil left a comment

Choose a reason for hiding this comment

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

LGTM unless squashing commits is not an option and this should be rebased without a merge commit.

@chadwhitacre
Copy link
Copy Markdown
Member

Hey it's green! 😁

@chadwhitacre chadwhitacre merged commit 3b5fe43 into getsentry:master Feb 15, 2022
@chadwhitacre
Copy link
Copy Markdown
Member

Thanks for your patience @lovetodream. 🙏

@github-actions github-actions Bot locked and limited conversation to collaborators Mar 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants