Skip to content

Conversation

@adriangl
Copy link
Contributor

@adriangl adriangl commented Jun 25, 2021

Github issue

Resolves #39

PR's key points

The PR migrates the API from v1 to v2, basically. Some tweaks had to be added to support the new API, but nothing note-worthy.

How to review this PR?

Test that your projects can be downloaded successfully with the new plug-in version

Definition of Done

  • Tests added (if new code is added)
  • There is no outcommented or debug code left
@adriangl adriangl self-assigned this Jun 25, 2021
@adriangl
Copy link
Contributor Author

@nokite could you please test if the implementation works? I've tested it with a few projects of my own, but just in case:

classpath "com.github.hyperdevs-team:poeditor-android-gradle-plugin:PR40-SNAPSHOT" 
Copy link
Contributor

@nokite nokite left a comment

Choose a reason for hiding this comment

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

Looks great!
I also tested it on my project and didn't find any issues.


companion object {
/** Returns the enum value associated to a string value. */
fun from(filterString: String) = valueOf(filterString.toUpperCase())
Copy link
Contributor

Choose a reason for hiding this comment

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

Just as a reminder (it's probably OK) - it will throw an IllegalArgumentException if the specified name does not match any of the enum constants.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it's intended :P. It may be a bit overkill to add a typed exception just for the use case, I think the IllegalArgumentException should give enough feedback about what's happening (which should be an incorrect value from parameters, I hope)

Regardless, thanks for pointing it out!

@adriangl adriangl force-pushed the task/poeditor-api-v2 branch from 0fd785f to 4c5e3ce Compare July 4, 2021 19:07
@adriangl adriangl merged commit 66a4285 into master Jul 4, 2021
@adriangl adriangl deleted the task/poeditor-api-v2 branch July 4, 2021 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

3 participants