Skip to content

Implement sgf-parsing practice exercise#2142

Merged
andrerfcsantos merged 9 commits intoexercism:mainfrom
tlphat:sgf-parsing
Aug 14, 2022
Merged

Implement sgf-parsing practice exercise#2142
andrerfcsantos merged 9 commits intoexercism:mainfrom
tlphat:sgf-parsing

Conversation

@tlphat
Copy link
Copy Markdown
Contributor

@tlphat tlphat commented Jul 15, 2022

pull request

Closes #1817. Provide test cases, starter, and reference implementations for the sgf-parsing exercise.

Note: The canonical data has been updated to remove the version (refer to this). The contributing doc should also be updated.


Reviewer Resources:

Track Policies

Copy link
Copy Markdown
Member

@andrerfcsantos andrerfcsantos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this! Looks good. Left some minor comments.

Comment thread config.json Outdated
Comment thread config.json
Comment thread exercises/practice/sgf-parsing/.meta/config.json Outdated
@andrerfcsantos andrerfcsantos added the x:rep/large Large amount of reputation label Jul 17, 2022
@tlphat tlphat requested a review from andrerfcsantos July 17, 2022 13:00
@tlphat
Copy link
Copy Markdown
Contributor Author

tlphat commented Jul 17, 2022

Hi @andrerfcsantos, I would be nice if you can take a look at my PR again. I'm a bit confused about the two workflows that failed (the gradle check and configlet lint). When I run those two tasks on my machine, it seems like there is a problem with the squeaky-clean concept and the file config.json.

@andrerfcsantos
Copy link
Copy Markdown
Member

The configlet check was failing because of the misconfiguration of the exercises, which was the target of my suggestions.

About the gradle error, now it is failing on line 118 in the file exercises/practice/sgf-parsing/.meta/src/reference/java/SgfParsing.java. I'm not certain why this is happening, I'll have to investigate further later if you don't catch the problem first.

@tlphat
Copy link
Copy Markdown
Contributor Author

tlphat commented Jul 18, 2022

Thanks for your information. It turns out that StringBuilder.isEmpty() had just been added since Java 15. I've changed it to a zero-length check. Hope that it'll work.

@andrerfcsantos
Copy link
Copy Markdown
Member

That was it :) The tests are now passing.

I'll do a final pass over this later.

@andrerfcsantos
Copy link
Copy Markdown
Member

I also created #2144 updating to Java 17. After that gets merged, we can try using StringBuilder.isEmpty() again.

Copy link
Copy Markdown
Member

@andrerfcsantos andrerfcsantos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's release this!

Thanks for the delay in reviewing it.

@andrerfcsantos andrerfcsantos merged commit 57afbd1 into exercism:main Aug 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

x:rep/large Large amount of reputation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sgf-parsing: implement exercise

2 participants