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.

Sign up for GitHub

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

Jump to bottom

[JavaToolInstallerV0] Add .dmg and .pkg support for macOS #13203

Merged
merged 4 commits into from
Jul 30, 2020
Merged

[JavaToolInstallerV0] Add .dmg and .pkg support for macOS #13203

merged 4 commits into from
Jul 30, 2020

Conversation

ekaterina-tatanova
Copy link
Contributor

@ekaterina-tatanova ekaterina-tatanova commented Jun 30, 2020 •
edited
Loading

Task name: JavaToolInstallerV0

Description: This PR adds .dmg and .pkg support for macOS.

Refactoring:

  • Changed the logic of function getting a supporting ending of file name
  • Changed the way of checking if the string is undefined
  • Added missing types to variables and functions results
  • Created a task utils module
  • Added JSDoc to functions
  • United two imports from the same module
  • Logic for unpacking the archive was extracted into a separate function

Documentation changes required: (Y)

Added unit tests: (N)

Checklist:

  • Task version was bumped - please check instruction how to do it
  • Checked that applied changes work as expected

@ekaterina-tatanova ekaterina-tatanova force-pushed the users/ekaterina-tatanova/feature1709682_dmg-and-pkg-support-for-macos branch from be9a82b to 9c3bfd6 Compare June 30, 2020 09:18
@ekaterina-tatanova ekaterina-tatanova changed the title Users/ekaterina tatanova/feature1709682 dmg and pkg support for macos [JavaToolInstallerV0] Add .dmg and .pkg support for macOS Jun 30, 2020
alexander-smolyakov
alexander-smolyakov approved these changes Jul 2, 2020
View reviewed changes
Copy link
Contributor

@alexander-smolyakov alexander-smolyakov left a comment

Choose a reason for hiding this comment

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

Please take a look at the comments, other changes are LGTM, thanks!

Tasks/JavaToolInstallerV0/javatoolinstaller.ts Outdated Show resolved Hide resolved
Tasks/JavaToolInstallerV0/task.loc.json Outdated Show resolved Hide resolved
Tasks/JavaToolInstallerV0/javatoolinstaller.ts Outdated Show resolved Hide resolved
Tasks/JavaToolInstallerV0/javatoolinstaller.ts Outdated Show resolved Hide resolved
anatolybolshakov
anatolybolshakov suggested changes Jul 3, 2020
View reviewed changes
Tasks/JavaToolInstallerV0/taskutils.ts Show resolved Hide resolved
Tasks/JavaToolInstallerV0/taskutils.ts Outdated Show resolved Hide resolved
Tasks/JavaToolInstallerV0/taskutils.ts Outdated Show resolved Hide resolved
Tasks/JavaToolInstallerV0/javatoolinstaller.ts Outdated Show resolved Hide resolved
Tasks/JavaToolInstallerV0/javatoolinstaller.ts Outdated Show resolved Hide resolved
Tasks/JavaToolInstallerV0/javatoolinstaller.ts Outdated Show resolved Hide resolved
@ekaterina-tatanova ekaterina-tatanova force-pushed the users/ekaterina-tatanova/feature1709682_dmg-and-pkg-support-for-macos branch from e10a341 to 7a4c920 Compare July 6, 2020 00:26
@anatolybolshakov anatolybolshakov mentioned this pull request Jul 6, 2020
Merged
2 tasks
@ekaterina-tatanova ekaterina-tatanova force-pushed the users/ekaterina-tatanova/feature1709682_dmg-and-pkg-support-for-macos branch from 500756d to 2cd2229 Compare July 6, 2020 11:51
@ekaterina-tatanova ekaterina-tatanova force-pushed the users/ekaterina-tatanova/feature1709682_dmg-and-pkg-support-for-macos branch 3 times, most recently from 4a6d4a2 to e2f2bf0 Compare July 6, 2020 15:46
anatolybolshakov
anatolybolshakov approved these changes Jul 8, 2020
View reviewed changes
Copy link
Contributor

@anatolybolshakov anatolybolshakov left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@leantk
Copy link
Contributor

leantk commented Jul 16, 2020

We should also make an issue to add canary tests for these new scenarios

@ekaterina-tatanova ekaterina-tatanova force-pushed the users/ekaterina-tatanova/feature1709682_dmg-and-pkg-support-for-macos branch from 9246469 to c065879 Compare July 17, 2020 09:03
@ekaterina-tatanova ekaterina-tatanova force-pushed the users/ekaterina-tatanova/feature1709682_dmg-and-pkg-support-for-macos branch from c065879 to 699ae5d Compare July 29, 2020 11:02
@ekaterina-tatanova ekaterina-tatanova force-pushed the users/ekaterina-tatanova/feature1709682_dmg-and-pkg-support-for-macos branch 2 times, most recently from 8564ac1 to 71c9863 Compare July 29, 2020 18:28
alexander-smolyakov
alexander-smolyakov reviewed Jul 30, 2020
View reviewed changes
Copy link
Contributor

@alexander-smolyakov alexander-smolyakov left a comment

Choose a reason for hiding this comment

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

Could you please take a look at the comments?
Other changes LGTM.

Tasks/JavaToolInstallerV0/javatoolinstaller.ts Outdated Show resolved Hide resolved
Tasks/JavaToolInstallerV0/taskutils.ts Outdated Show resolved Hide resolved
Tasks/JavaToolInstallerV0/taskutils.ts Outdated Show resolved Hide resolved
egor-bryzgalov
egor-bryzgalov approved these changes Jul 30, 2020
View reviewed changes
Copy link
Member

@egor-bryzgalov egor-bryzgalov left a comment

Choose a reason for hiding this comment

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

LGTM

@ekaterina-tatanova ekaterina-tatanova force-pushed the users/ekaterina-tatanova/feature1709682_dmg-and-pkg-support-for-macos branch from 71c9863 to 96d2554 Compare July 30, 2020 10:53
alexander-smolyakov
alexander-smolyakov approved these changes Jul 30, 2020
View reviewed changes
Copy link
Contributor

@alexander-smolyakov alexander-smolyakov left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

@ekaterina-tatanova ekaterina-tatanova merged commit 431cd6d into master Jul 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anatolybolshakov anatolybolshakov anatolybolshakov approved these changes

@leantk leantk leantk approved these changes

@alexander-smolyakov alexander-smolyakov alexander-smolyakov approved these changes

@egor-bryzgalov egor-bryzgalov egor-bryzgalov approved these changes

@AlexandrKravchuk AlexandrKravchuk Awaiting requested review from AlexandrKravchuk

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ekaterina-tatanova @leantk @anatolybolshakov @alexander-smolyakov @egor-bryzgalov