Skip to content

Add tests for already fixed issues #649

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

Merged
merged 1 commit into from
Aug 30, 2022

Conversation

omochi
Copy link
Contributor

@omochi omochi commented Aug 29, 2022

I started to fix issues #632 and #634.
But it seems to be already fixed.

I added test cases for them.

@omochi omochi requested a review from ahoppen as a code owner August 29, 2022 03:30
@ahoppen
Copy link
Member

ahoppen commented Aug 29, 2022

Oh, nice. Those are always the best issues that have already been fixed. 👍

I just merged #641 that changes the allowErrors parameter of AssertParse to instead require specifying the produced diagnostics. Could you update your PR for that API?

You don’t need to worry about the quality of the generated diagnostics at this point, I’ll do some basic work on general diagnostic quality in the week(s). If you think the diagnostics are sub-optimal, just add a FIXME for now.

@omochi
Copy link
Contributor Author

omochi commented Aug 29, 2022

OK, I edit my PR to use new checking for specific diagnostics.

@omochi
Copy link
Contributor Author

omochi commented Aug 30, 2022

@ahoppen I updated PR.
In both cases, diagnostics from parser is weird.
I wrote FIXME about it.

@omochi omochi force-pushed the add-fixed-issues-test branch from 6b11622 to c6e1431 Compare August 30, 2022 04:17
@CodaFi
Copy link
Contributor

CodaFi commented Aug 30, 2022

@swift-ci test

@ahoppen ahoppen merged commit a36ed26 into swiftlang:main Aug 30, 2022
@omochi omochi deleted the add-fixed-issues-test branch August 30, 2022 16:22
This was referenced Aug 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants