-
-
Notifications
You must be signed in to change notification settings - Fork 57
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
[WIP] Optimistic download #184
base: master
Are you sure you want to change the base?
Conversation
…om/blender/Rome into feature/issue-175-role-arn-support
Generated by 🚫 Danger |
e9a3d00
to
66ed491
Compare
@blender Thanks for this work, it will certainly save us a lot of time... would you be able to make a pre-release binary (or point me in the right direction) so I can test it and provide feedback? |
Hi,
I am not close to my computer at the moment. I will make a pre release on Monday.
If you feel adventurous you can try to compile yourself. Instructions are in the read me.
…Sent from my iPhone
On 31 May 2019, at 17:21, Oliver Atkinson ***@***.***> wrote:
@blender Thanks for this work, it will certainly save us a lot of time... would you be able to make a pre-release binary with this so I can test it and provide feedback?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Thanks for the pointer @blender - I got the branch compiled, but it did not do what I expect. I assumed it would do the following:
I was not seeing the above behaviour, all files relating to the frameworks with valid version files were still being downloaded. However, there's a good chance I did not set it up correctly as I'm new to Rome. edit:
I have just seen your comment on the original issue, maybe this is what I am seeing - is it safe to add an option which does not download those files if the version file is present? |
Did you run it with —optimistic ?
…Sent from my iPhone
On 1 Jun 2019, at 14:04, Oliver Atkinson ***@***.***> wrote:
Thanks for the pointer @blender - I got the branch compiled, but it did not do what I expect. I assumed it would do the following:
Check the Cartfile.resolved against the .version file
If a match, no files relating to the framework would be requested (including bcsymbolmaps)
If there's a version mismatch, or lack of a version file then it would download all of the files relating to the framework.
I was not seeing the above behaviour, all files relating to the frameworks with valid version files were still being downloaded.
However, there's a good chance I did not set it up correctly as I'm new to Rome.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Yes |
This needs tests if anyone looking for hacktoberfest issues wants to finish it |
@@ -96,6 +103,7 @@ udcPayloadParser = | |||
<*> noIgnoreParser | |||
<*> noSkipCurrentParser | |||
<*> concurrentlyParser | |||
<*> optimisticParser | |||
|
|||
uploadParser :: Opts.Parser RomeCommand | |||
uploadParser = pure Upload <*> udcPayloadParser |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found Use <$>
Why Not
|
reverseRomeMap | ||
fVersion | ||
platform | ||
flip whenLeft eitherSymbolmapsOrErrors $ \e -> case e of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found Use lambda-case
Why Not
|
Any progress on this PR? |
@vytautasgimbutas it needs to be adapted to the current code base and it needs integration tests. Do you want to give it a shot? |
I'd be very interested in this too. |
When this is made available, can you also please include the "optimistic" parameter in the fasltane rome plugin? |
Contributions are accepted @felipe-azevedo |
As per #181 , if a version file exists trust that everything else is there too.