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

Clean util_string #2339

Merged
merged 4 commits into from
Mar 3, 2023
Merged

Clean util_string #2339

merged 4 commits into from
Mar 3, 2023

Conversation

AntoinePrv
Copy link
Member

@AntoinePrv AntoinePrv commented Mar 1, 2023

  • Clean up the util_string header
  • Add util_string.cpp
  • Decouple util.hpp from util_string.hpp (lots of header changes related to this)
  • Add strip_if function (needed for following PR) Done in Add strip_if functions #2344
  • Add split_once functions (needed for following PR) Done in Add split_once functions #2345
  • Add split_for_each as a replacement of split?

@AntoinePrv AntoinePrv self-assigned this Mar 1, 2023
@AntoinePrv AntoinePrv force-pushed the util-string branch 3 times, most recently from 337d7fd to 031b380 Compare March 2, 2023 13:35
@AntoinePrv AntoinePrv marked this pull request as ready for review March 3, 2023 08:41
@AntoinePrv
Copy link
Member Author

Can be merged anytime to avoid conflicts, the rest can be merged in another PR.

@AntoinePrv AntoinePrv changed the title Clean and extend util_string Clean util_string Mar 3, 2023
@AntoinePrv AntoinePrv mentioned this pull request Mar 3, 2023
1 task
Copy link
Member

@Hind-M Hind-M left a comment

Choose a reason for hiding this comment

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

This PR is so long :) I expect the build/tests to complain if there is any problem, otherwise, I guess we will find out soon!

@@ -290,7 +292,7 @@ namespace mamba
}

LOG_DEBUG << "Currently running processes: " << get_all_running_processes_info();
LOG_DEBUG << "Remaining args to run as command: " << join(" ", command);
fmt::print(LOG_DEBUG, "Remaining args to run as command: {}", fmt::join(command, " "));
Copy link
Member

Choose a reason for hiding this comment

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

This is more a general question, but are we using fmt now everywhere for logging?
(btw, the LOG_DEBUG in the line above could be replaced as well)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we can use fmt wherever we need it, here it avoids allocating the result of join.

libmamba/src/core/util_string.cpp Outdated Show resolved Hide resolved
libmamba/include/mamba/core/util_string.hpp Outdated Show resolved Hide resolved
libmamba/include/mamba/core/util_string.hpp Outdated Show resolved Hide resolved
libmamba/include/mamba/core/util_string.hpp Show resolved Hide resolved
libmamba/include/mamba/core/util_string.hpp Show resolved Hide resolved
@JohanMabille JohanMabille merged commit 2811c25 into mamba-org:main Mar 3, 2023
@AntoinePrv AntoinePrv deleted the util-string branch March 3, 2023 17:01
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.

None yet

3 participants