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

Fixed struct array initialization #264 #283

Merged
merged 3 commits into from
Oct 30, 2017
Merged

Fixed struct array initialization #264 #283

merged 3 commits into from
Oct 30, 2017

Conversation

yulvil
Copy link
Contributor

@yulvil yulvil commented Oct 28, 2017

Reverted code from newDeclStmt and moved the array initialization
logic to the transpileInitListExpr function

Ref: #264


This change is Reviewable

Reverted code from newDeclStmt and moved the array initialization
logic to the transpileInitListExpr function
@codecov
Copy link

codecov bot commented Oct 28, 2017

Codecov Report

Merging #283 into master will increase coverage by <.01%.
The diff coverage is 90.9%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #283      +/-   ##
==========================================
+ Coverage   81.78%   81.78%   +<.01%     
==========================================
  Files         136      136              
  Lines        5513     5524      +11     
==========================================
+ Hits         4509     4518       +9     
- Misses        802      803       +1     
- Partials      202      203       +1
Impacted Files Coverage Δ
transpiler/transpiler.go 91.66% <100%> (+0.07%) ⬆️
ast/init_list_expr.go 89.47% <100%> (+0.58%) ⬆️
transpiler/variables.go 82.82% <89.28%> (-0.41%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a24a0df...e6fdff0. Read the comment docs.

@yulvil
Copy link
Contributor Author

yulvil commented Oct 28, 2017

@elliotchance Let me know if there is anything to change.

@elliotchance
Copy link
Owner

Reviewed 5 of 5 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


ast/init_list_expr_test.go, line 15 at r1 (raw file):

			ChildNodes: []Node{},
		},
		`InitListExpr 0x32017f0 <col:24, col:41> 'struct node [2]'`: &InitListExpr{

I'm surprised this actually worked (matched the regex) with the InitListExpr prefix. Please remove that prefix so it is the same as all the rest of our tests (and the one below).


Comments from Reviewable

@yulvil
Copy link
Contributor Author

yulvil commented Oct 30, 2017

Review status: 3 of 5 files reviewed at latest revision, 1 unresolved discussion.


ast/init_list_expr_test.go, line 15 at r1 (raw file):

Previously, elliotchance (Elliot Chance) wrote…

I'm surprised this actually worked (matched the regex) with the InitListExpr prefix. Please remove that prefix so it is the same as all the rest of our tests (and the one below).

Done.


Comments from Reviewable

@elliotchance
Copy link
Owner

Reviewed 2 of 2 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@elliotchance elliotchance merged commit 6890f1a into elliotchance:master Oct 30, 2017
@yulvil yulvil deleted the arrayinit branch October 30, 2017 04:34
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