Skip to content

Switch to assertj for exceptions#2502

Merged
sanderploegsma merged 33 commits into
exercism:mainfrom
keszocze:switch-to-assertj-for-exceptions
Sep 29, 2023
Merged

Switch to assertj for exceptions#2502
sanderploegsma merged 33 commits into
exercism:mainfrom
keszocze:switch-to-assertj-for-exceptions

Conversation

@keszocze
Copy link
Copy Markdown
Contributor

@keszocze keszocze commented Sep 28, 2023

pull request

Switching to AssertJ for the

  • Forth
  • Ledger
  • OCR
  • Palindrom Calculator
  • Phone Number
  • SGF Parsing
  • Simple Linked List
  • Triangle
    exercises.

This request brings #2147 closer to being fully solved.


Reviewer Resources:

Track Policies

@sanderploegsma
Copy link
Copy Markdown
Contributor

It looks like this includes updates to error-handling, which was already merged. Could you rebase your branch on exercism:main?

@keszocze
Copy link
Copy Markdown
Contributor Author

The change is very tiny in the sense that I only make the imports explicit. I probably can rebase it but have to look up what that is and how it works. I am wondering whether that really is worth the hassle, though.

@sanderploegsma
Copy link
Copy Markdown
Contributor

Ah well the reason I mentioned it is because there are conflicts with the main branch.

If rebasing is new for you it’s also fine if you just merge the main branch into yours and resolve the conflicts that way 👍

@sanderploegsma
Copy link
Copy Markdown
Contributor

sanderploegsma commented Sep 29, 2023

@keszocze thanks for picking up all of these! I'd like to ask you to make additional updates in a different PR: updates to exercise tests trigger new test runs of submitted solutions on the Exercism infrastructure, so we try not to update too many at once at risk of overloading the servers.

Plus, small and focused PRs make my job of reviewing them a bit more manageable 🙃

@keszocze
Copy link
Copy Markdown
Contributor Author

Sure can do that. Then I stop adding more to this PR. As all changes belong to a single issue, I thought that this was fine. I sure do not want to make your life unnecessarily complicated ;)

Copy link
Copy Markdown
Contributor

@sanderploegsma sanderploegsma left a comment

Choose a reason for hiding this comment

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

Nice work so far! I have left a couple of comments for you to address before we can merge this.

Comment thread exercises/practice/custom-set/src/test/java/CustomSetTest.java Outdated
Comment thread exercises/practice/custom-set/src/test/java/CustomSetTest.java Outdated
Comment thread exercises/practice/ledger/src/test/java/LedgerTest.java Outdated
Comment thread exercises/practice/sgf-parsing/src/test/java/SgfParsingTest.java Outdated
Comment thread exercises/practice/sgf-parsing/src/test/java/SgfParsingTest.java Outdated
Copy link
Copy Markdown
Contributor

@sanderploegsma sanderploegsma left a comment

Choose a reason for hiding this comment

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

All good, thanks for contributing!

@sanderploegsma sanderploegsma merged commit 529b75a into exercism:main Sep 29, 2023
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