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

ci: Get rid of binaries submodule #1385

Merged
merged 15 commits into from
Dec 14, 2020
Merged

Conversation

piotradamczyk5
Copy link
Contributor

@piotradamczyk5 piotradamczyk5 commented Dec 8, 2020

Fixes #896

This PR delete submodule for binaries and download them at runtime from release page based on os system

Test Plan

How do we know the code works?

macOs -> all should work by default because previously binaries were not copied on macOS
linux -> it download binaries from release page at runtime to <user_home>/.flank directory and unpack them
windows -> it works similar to linux version, however, full support will be available after #1134

All tests should work properly

Checklist

  • Documented
  • Unit tested
  • Downloading binaries for Linux and Windows at runtime
  • Create binaries release for Linux and Windows
  • Update binaries assets after each change on binaries repo release page >> PR
  • Delete submodule
  • Update Github Actions to do not checkout submodules

@codecov-io
Copy link

codecov-io commented Dec 8, 2020

Codecov Report

Merging #1385 (6f53eb2) into master (354213d) will decrease coverage by 0.32%.
The diff coverage is 23.52%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1385      +/-   ##
============================================
- Coverage     77.58%   77.26%   -0.33%     
  Complexity      717      717              
============================================
  Files           248      250       +2     
  Lines          4788     4825      +37     
  Branches        915      922       +7     
============================================
+ Hits           3715     3728      +13     
- Misses          572      596      +24     
  Partials        501      501              

@piotradamczyk5 piotradamczyk5 force-pushed the #896_get_rid_of_submodules branch from e659ccd to bb706b5 Compare December 9, 2020 14:00
@jan-goral jan-goral self-assigned this Dec 9, 2020
@piotradamczyk5 piotradamczyk5 force-pushed the #896_get_rid_of_submodules branch from c0de1a3 to bb706b5 Compare December 9, 2020 15:52
@github-actions
Copy link
Contributor

github-actions bot commented Dec 10, 2020

Timestamp: 2020-12-14 14:23:39
Buildscan url for ubuntu-workflow run 420943162
https://gradle.com/s/h57ie6s6knq44

@piotradamczyk5 piotradamczyk5 marked this pull request as ready for review December 10, 2020 16:25
@piotradamczyk5 piotradamczyk5 marked this pull request as draft December 11, 2020 06:49
@piotradamczyk5 piotradamczyk5 force-pushed the #896_get_rid_of_submodules branch from 18d7b0b to 17ed36e Compare December 11, 2020 06:54
@piotradamczyk5 piotradamczyk5 force-pushed the #896_get_rid_of_submodules branch from 6e76562 to 93f9cbd Compare December 11, 2020 17:23
@piotradamczyk5 piotradamczyk5 marked this pull request as ready for review December 11, 2020 18:03
1. checkout binaries [repository](https://github.com/Flank/binaries)
1. update them using:
- `updateBinariesWithFlankBash` will update binaries for Linux and Windows using `flank-scripts`
- `update.sh` (old method). It will update binaries for Linux OS
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need the old method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is standalone without flank-scripts needed

Copy link
Contributor

@jan-goral jan-goral left a comment

Choose a reason for hiding this comment

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

The code looks well, but I would love to have this implementation in the java-library module linked to the test_runner as Gradle dependency, maybe not now, it could be in another PR.
Some of the code looks a little bit redundant according to flank-scripts, for example, utilities for unzipping & symlinks, probably also could be moved to another library module shared between download-binaries(?) and flank-scripts.

@piotradamczyk5
Copy link
Contributor Author

The code looks well, but I would love to have this implementation in the java-library module linked to the test_runner as Gradle dependency, maybe not now, it could be in another PR.
Some of the code looks a little bit redundant according to flank-scripts, for example, utilities for unzipping & symlinks, probably also could be moved to another library module shared between download-binaries(?) and flank-scripts.

Yeah I think that we could do some shared module which has common logic for other modules, I will create a ticket for it

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.

  • Tested on windows, works!

@piotradamczyk5 piotradamczyk5 merged commit fd2c0a6 into master Dec 14, 2020
@piotradamczyk5 piotradamczyk5 deleted the #896_get_rid_of_submodules branch December 14, 2020 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move binaries from resources to external repo (or other place)
4 participants