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

test case for #9180 and re-enables the disabled tcompilerapi test #9181

Merged
merged 2 commits into from
Oct 11, 2018

Conversation

timotheecour
Copy link
Member

@timotheecour timotheecour commented Oct 4, 2018

/cc @Araq

Note: #9184 is still an issue after this PR (no change before / after this PR regarding that issue)

@alaviss
Copy link
Collaborator

alaviss commented Oct 4, 2018

Instead of implementing more workaround, could you try to solve the root of this issue? #9068

@timotheecour
Copy link
Member Author

Instead of implementing more workaround, could you try to solve the root of this issue? #9068

can be done in a subsequent PR by me or someone else. I prefer incremental progress to no progress ;-)

@timotheecour timotheecour changed the title fix #9180 fix #9180 and re-enables the disabled tcompilerapi Oct 4, 2018
@timotheecour timotheecour changed the title fix #9180 and re-enables the disabled tcompilerapi fix #9180 and re-enables the disabled tcompilerapi test Oct 4, 2018
@dom96
Copy link
Contributor

dom96 commented Oct 4, 2018

I think this would have been much better as two PRs.

@timotheecour
Copy link
Member Author

@dom96 would u accept:

  • 1 PR that adds findNimStdLibCompileTime and enables tcompilerapi test which was effectively disabled

  • 1 PR that does the rest as mentioned here

@dom96
Copy link
Contributor

dom96 commented Oct 4, 2018

You might as well keep it as-is now, since you've already created it. In the future consider creating two PRs.

As far as accepting this: that's @Araq's turf.

@timotheecour
Copy link
Member Author

PTAL

@timotheecour
Copy link
Member Author

timotheecour commented Oct 9, 2018

@alaviss

Instead of implementing more workaround, could you try to solve the root of this issue? #9068

issue still there even after #9068 was fixed; this PR should fix this issue. EDIT: hold on

@timotheecour
Copy link
Member Author

timotheecour commented Oct 9, 2018

PTAL: rebased PR after removing a commit that added workaround for #9068 now that it was fixed in #9259

@timotheecour timotheecour changed the title fix #9180 and re-enables the disabled tcompilerapi test test case for #9180 and re-enables the disabled tcompilerapi test Oct 10, 2018
@Araq Araq merged commit a58c982 into nim-lang:devel Oct 11, 2018
@timotheecour timotheecour deleted the pr_fix_9180 branch October 11, 2018 07:53
krux02 pushed a commit to krux02/Nim that referenced this pull request Oct 15, 2018
…test (nim-lang#9181)

* add findNimStdLibCompileTime and un-disable tcompilerapi test; add test case for nim-lang#9180

* address comments
narimiran pushed a commit to narimiran/Nim that referenced this pull request Oct 31, 2018
…test (nim-lang#9181)

* add findNimStdLibCompileTime and un-disable tcompilerapi test; add test case for nim-lang#9180

* address comments
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.

compiler/nimeval can't be used twice: fails 2nd time with: Error: internal error: n is not nil
4 participants