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

Download #2844

Merged
merged 8 commits into from
Sep 29, 2023
Merged

Download #2844

merged 8 commits into from
Sep 29, 2023

Conversation

JohanMabille
Copy link
Member

@JohanMabille JohanMabille commented Sep 18, 2023 •
edited
Loading

  • Slight refactor of Downloader APIs
  • Wrapped progress bars in a monitoring class
  • Reworked MSubdirMetadata class
  • Plugged new downloader in MSubdirData
  • Restricted MSubdirData API

This PR is backward incompatible.

N.B.: the file fetch.hpp and fetch.cpp will be removed in a future PR where the new downloader is plugged into the package downloader.

@JohanMabille JohanMabille force-pushed the download branch 6 times, most recently from 31ce0ba to 973045c Compare September 25, 2023 07:37
@JohanMabille JohanMabille force-pushed the download branch 3 times, most recently from 197bf53 to 84dd66a Compare September 26, 2023 04:07
@JohanMabille JohanMabille force-pushed the download branch 3 times, most recently from 82cc655 to 43488f7 Compare September 26, 2023 04:44
AntoinePrv
AntoinePrv approved these changes Sep 26, 2023
View reviewed changes
Copy link
Member

@AntoinePrv AntoinePrv left a comment

Choose a reason for hiding this comment

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

A first pass, tough to read through all of it

libmamba/include/mamba/core/download.hpp Outdated Show resolved Hide resolved
libmamba/include/mamba/core/download_progress_bar.hpp Outdated Show resolved Hide resolved
libmamba/include/mamba/core/subdirdata.hpp Outdated Show resolved Hide resolved
libmamba/src/core/subdirdata.cpp Show resolved Hide resolved
libmamba/src/core/subdirdata.cpp Outdated Show resolved Hide resolved
libmamba/src/core/subdirdata.cpp Outdated Show resolved Hide resolved
libmamba/tests/src/core/test_cpp.cpp Outdated Show resolved Hide resolved
libmamba/tests/src/core/test_transfer.cpp Outdated Show resolved Hide resolved
libmamba/src/core/download.cpp Show resolved Hide resolved
libmamba/src/core/download.cpp Outdated Show resolved Hide resolved
Klaim
Klaim reviewed Sep 28, 2023
View reviewed changes
libmamba/src/core/download.cpp Outdated Show resolved Hide resolved
libmamba/src/core/download.cpp Show resolved Hide resolved
libmamba/src/core/download_impl.hpp Show resolved Hide resolved
libmamba/src/core/download_progress_bar.cpp Outdated Show resolved Hide resolved
libmamba/src/core/download_progress_bar.cpp Outdated Show resolved Hide resolved
libmamba/src/core/subdirdata.cpp Outdated Show resolved Hide resolved
libmamba/src/core/subdirdata.cpp Outdated Show resolved Hide resolved
libmamba/src/core/subdirdata.cpp Outdated Show resolved Hide resolved
libmamba/src/core/subdirdata.cpp Outdated Show resolved Hide resolved
libmambapy/src/main.cpp Outdated Show resolved Hide resolved
@JohanMabille JohanMabille force-pushed the download branch 2 times, most recently from 7424376 to e099908 Compare September 29, 2023 11:49
Klaim
Klaim approved these changes Sep 29, 2023
View reviewed changes
Copy link
Member

@Klaim Klaim left a comment •
edited
Loading

Choose a reason for hiding this comment

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

There is still some const missing (that I didnt report here) but nothing critical, LGTM otherwise.

libmamba/src/core/download_progress_bar.cpp Show resolved Hide resolved
libmamba/src/core/subdirdata.cpp Outdated Show resolved Hide resolved
@JohanMabille JohanMabille merged commit 474be8b into mamba-org:main Sep 29, 2023
@JohanMabille JohanMabille deleted the download branch September 29, 2023 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Hind-M Hind-M Hind-M left review comments

@wolfv wolfv wolfv left review comments

@AntoinePrv AntoinePrv AntoinePrv approved these changes

@Klaim Klaim Klaim approved these changes

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
@JohanMabille @Klaim @wolfv @AntoinePrv @Hind-M