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

refactor: Remove client classes for JUnit refactor #1911

Merged
merged 2 commits into from
May 11, 2021

Conversation

piotradamczyk5
Copy link
Contributor

@piotradamczyk5 piotradamczyk5 commented May 10, 2021

Test Plan

How do we know the code works?

This PR contains additional fixes for JUnit data layer refactor after conversation with @jan-gogo on Slack

Checklist

  • Unit tested

@github-actions
Copy link
Contributor

github-actions bot commented May 10, 2021

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@github-actions
Copy link
Contributor

github-actions bot commented May 10, 2021

Timestamp: 2021-05-11 17:36:32
Buildscan url for ubuntu-workflow run 832572914
https://gradle.com/s/bm2hgh6rtlewa

import ftl.client.google.downloadAsJunitXml

object DownloadAsJunitXML :
FileReference.DownloadAsXML,
(FileReference) -> JUnitTest.Result by { fileReference ->
downloadAsJunitXml(fileReference).toApiModel()
Copy link
Contributor

Choose a reason for hiding this comment

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

@piotradamczyk5 I am a little confused as the removal of .toApiModel. Whats this change for? Or why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jan-gogo suggest that we should use api data classes also in client, because they are small

@piotradamczyk5 piotradamczyk5 force-pushed the fixes_for_junit_refactor branch from 0ff7ae8 to 884315a Compare May 11, 2021 11:17
adamfilipow92
adamfilipow92 previously approved these changes May 11, 2021
Copy link
Contributor

@adamfilipow92 adamfilipow92 left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@adamfilipow92 adamfilipow92 self-requested a review May 11, 2021 11:35
@adamfilipow92 adamfilipow92 dismissed their stale review May 11, 2021 11:40

sorry wrong pull request

@bootstraponline bootstraponline force-pushed the fixes_for_junit_refactor branch from 5c1069a to 88a2647 Compare May 11, 2021 11:55
Copy link
Contributor

@adamfilipow92 adamfilipow92 left a comment

Choose a reason for hiding this comment

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

  • Code looks good
  • All test pass

@piotradamczyk5 piotradamczyk5 enabled auto-merge (squash) May 11, 2021 13:11
@piotradamczyk5 piotradamczyk5 requested a review from Sloox May 11, 2021 13:11
@bootstraponline bootstraponline force-pushed the fixes_for_junit_refactor branch 2 times, most recently from 8da299c to 070e5e7 Compare May 11, 2021 17:30
@bootstraponline bootstraponline force-pushed the fixes_for_junit_refactor branch from 070e5e7 to ee0a9d9 Compare May 11, 2021 17:46
@piotradamczyk5 piotradamczyk5 merged commit ba61ffa into master May 11, 2021
@piotradamczyk5 piotradamczyk5 deleted the fixes_for_junit_refactor branch May 11, 2021 18:34
@github-actions github-actions bot locked and limited conversation to collaborators May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants