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

Remove unnecessary try/catch per #17729 #17823

Merged
merged 1 commit into from
Jul 16, 2020

Conversation

colemanw
Copy link
Member

Overview

Now that #17729 is merged the API will automatically guard against querying unknown fields, we can remove this precaution.

Before

Try/catch block.

After

There is no try.
There is no try

Comments

Thanks to @demeritcowboy I can't get this meme out of my head.

@civibot
Copy link

civibot bot commented Jul 14, 2020

(Standard links)

@eileenmcnaughton
Copy link
Contributor

I think the upgrade tests should give adequate cover on this @seamuslee001 ? Also the replacement fix has tests.

I'm +1 on this

@eileenmcnaughton eileenmcnaughton added the merge ready PR will be merged after a few days if there are no objections label Jul 14, 2020
@eileenmcnaughton
Copy link
Contributor

test fail not related

@seamuslee001
Copy link
Contributor

Looks fine to me merging

@seamuslee001 seamuslee001 merged commit c1e4d09 into civicrm:master Jul 16, 2020
@seamuslee001 seamuslee001 deleted the upgradeSafe branch July 16, 2020 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master merge ready PR will be merged after a few days if there are no objections ok-without-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants