Skip to content

Conversation

jagdish-15
Copy link
Member

Pull Request

This PR addresses issue #2896 by updating the method signature in the starter code. Additionally, it updates the example solution to align with the revised method signature in the starter code.


Reviewer Resources:

Track Policies

Copy link
Member

@kahgoh kahgoh left a comment

Choose a reason for hiding this comment

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

Hey @jagdish-15, I've just had a look and think this looks good. I just need a bit of time to see if this can work with List<?> flatten(List<?> list) instead of List<Object> flatten(List<?> list).

@jagdish-15
Copy link
Member Author

Hey @kahgoh, thanks for taking a look. I totally get that you want to check if List<?> flatten(List<?> list) can work instead of List<Object> flatten(List<?> list). From what I saw, the wildcard type (?) was causing some issues with the assertions.

Feel free to take your time, and let me know if you need anything from my side! Looking forward to hearing your thoughts.

Copy link
Member

@kahgoh kahgoh left a comment

Choose a reason for hiding this comment

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

Hey @jagdish-15, after spending time looking at trying to change to the List<?> flatten(List<?>) I think we should stick with what you have for now.

The only way (to change it) I could think of is to change the assertion to isEqualTo(expected) list. The downside with this is that isEqualTo doesn't provide as much information as containsExactly (containsExactly can tell you what it couldn't find or what it didn't expect, whereas isEqualTo will only tell you the lists aren't equal). I like the extra information from the containsExactly.

I'll approve and merge this one.

@kahgoh kahgoh merged commit 406af2a into exercism:main Jan 14, 2025
4 checks passed
@jagdish-15
Copy link
Member Author

@kahgoh, I noticed that the config.json file for this exercise lists 21 contributors, but only 17 are visible on the website. Initially, I thought this issue affected only me, but I now realize it’s consistent. What could be causing this discrepancy?

@kahgoh
Copy link
Member

kahgoh commented Jan 15, 2025

Recently, there was a problem with the Github actions that the CI relies on. I had noticed they failed after I merged it. I was going to see if this gets fixed with the next run when the next PR is merged.

1 similar comment
@kahgoh
Copy link
Member

kahgoh commented Jan 15, 2025

Recently, there was a problem with the Github actions that the CI relies on. I had noticed they failed after I merged it. I was going to see if this gets fixed with the next run when the next PR is merged.

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