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

fix sdl_test test that failed in CI cpp mode #10314

Merged
merged 2 commits into from
Jan 15, 2019

Conversation

timotheecour
Copy link
Member

@timotheecour timotheecour commented Jan 15, 2019

note

the fixed code now matches the declaration of drawLines which expects cint, not int32

proc drawLines*(renderer: SdlRendererPtr, points: ptr tuple[x, y: cint],
    count: cint): cint {.importc: "SDL_RenderDrawLines", discardable.}

note that cint isnt' defined by type cint = int32 but as follows:

type cint* {.importc: "int", nodecl.} = int32

note

after #10313 and #10310 there will be 1 last failing tests before we can enable nim cpp in CI:

  • tests/objects/tobjcov.nim

@Araq
Copy link
Member

Araq commented Jan 15, 2019

That's not good, the point of these tests is that they continue to work. I think instead we should compile "Nim in Action" examples always in C mode.

@timotheecour
Copy link
Member Author

but cpp is intended to be future default IIRC. i'll instead use when defined(cpp) block and use old impl for c

@Araq
Copy link
Member

Araq commented Jan 15, 2019

These example programs should only be touched if absolutely necessary.

@timotheecour timotheecour force-pushed the pr_fix_sdl_test_ci_cpp branch from 17e61e2 to 45841ec Compare January 15, 2019 21:53
@timotheecour
Copy link
Member Author

PTAL

That's not good, the point of these tests is that they continue to work

disabling cpp mode because they fail in nim cpp mode goes in the opposite direction of making sure these tests continue to work.
I just pushed a commit that'll run these tests exactly the way they did in NimInAction book when in C mode, while still making sure that example stays supported in cpp mode so a user inspired to port/adapt that example to nim cpp will have an easier time.

@Araq
Copy link
Member

Araq commented Jan 15, 2019

Not what I had in mind, but alright.

@Araq Araq added the merge_when_passes_CI mergeable once green label Jan 15, 2019
@timotheecour timotheecour merged commit fbd6743 into nim-lang:devel Jan 15, 2019
@timotheecour timotheecour deleted the pr_fix_sdl_test_ci_cpp branch January 15, 2019 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants