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

Adding FStar.Printable.fst with test by proof in tests/printable usin… #2902

Merged
merged 5 commits into from
May 2, 2023
Merged

Adding FStar.Printable.fst with test by proof in tests/printable usin… #2902

merged 5 commits into from
May 2, 2023

Conversation

briangmilnes
Copy link
Contributor

No description provided.

@mtzguido
Copy link
Member

mtzguido commented May 1, 2023

Hi Brian, thanks! It mostly looks good, I just have one comment. There's no need to make this tests/printable directory, nor check F*'s output. Just putting this test file in, say, tests/micro-benchmarks is enough to get into the existing rules which will check that everything succeeds.

Could you make that change? Simply pushing to your branch will update the PR accordingly.

@briangmilnes
Copy link
Contributor Author

briangmilnes commented May 1, 2023 via email

@mtzguido
Copy link
Member

mtzguido commented May 1, 2023

Ah, I see. Could we anyway make that a separate PR, and not block this one on the unit testing stuff?

@briangmilnes
Copy link
Contributor Author

briangmilnes commented May 1, 2023 via email

@briangmilnes
Copy link
Contributor Author

Guido, TestPrintable moved to micro-benchmarks. All tests completed.

@mtzguido
Copy link
Member

mtzguido commented May 1, 2023

Hi Brian, I see you've merged master, but I don't see any other changes. Did you maybe forget to push?

@briangmilnes
Copy link
Contributor Author

briangmilnes commented May 1, 2023 via email

@mtzguido
Copy link
Member

mtzguido commented May 1, 2023

I see it now! Just please also remove the change to tests/Makefile

@briangmilnes
Copy link
Contributor Author

briangmilnes commented May 1, 2023 via email

@mtzguido mtzguido merged commit 9eeb519 into FStarLang:master May 2, 2023
@mtzguido
Copy link
Member

mtzguido commented May 2, 2023

Thanks Brian, just merged this.

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.

2 participants