From dee65a91fbdbb4b2011c079da5dd060b200d4586 Mon Sep 17 00:00:00 2001 From: AmirMS <104940545+AmelBawa-msft@users.noreply.github.com> Date: Wed, 11 Dec 2024 17:18:50 -0800 Subject: [PATCH] Addressing comments --- src/AppInstallerCommonCore/Experiment.cpp | 16 ++++++++-------- .../Public/winget/Experiment.h | 12 ++++++------ src/AppInstallerSharedLib/JsonUtil.cpp | 5 +++-- .../Public/winget/JsonUtil.h | 2 +- 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/src/AppInstallerCommonCore/Experiment.cpp b/src/AppInstallerCommonCore/Experiment.cpp index 37bff5e704..e7f12a8b15 100644 --- a/src/AppInstallerCommonCore/Experiment.cpp +++ b/src/AppInstallerCommonCore/Experiment.cpp @@ -13,19 +13,19 @@ namespace AppInstaller::Settings { ExperimentState GetExperimentStateInternal(Experiment::Key key, const UserSettings& userSettings) { - if (key == Experiment::Key::None) - { - return { true, ExperimentToggleSource::Default }; - } - - auto experiment = Experiment::GetExperiment(key); if (!GroupPolicies().IsEnabled(TogglePolicy::Policy::Experiments)) { - AICLI_LOG(Core, Info, << "Experiment " << experiment.Name() << + AICLI_LOG(Core, Info, << "Experiment " << Experiment::GetExperiment(key).Name() << " is disabled due to group policy: " << TogglePolicy::GetPolicy(TogglePolicy::Policy::Experiments).RegValueName()); return { false, ExperimentToggleSource::Policy }; } + if (key == Experiment::Key::None) + { + return { false, ExperimentToggleSource::Default }; + } + + auto experiment = Experiment::GetExperiment(key); auto userSettingsExperiments = userSettings.Get(); auto jsonName = std::string(experiment.JsonName()); if (userSettingsExperiments.find(jsonName) != userSettingsExperiments.end()) @@ -80,7 +80,7 @@ namespace AppInstaller::Settings switch (key) { case Key::CDN: - return Experiment{ "CDN experiment", "CDN", "https://aka.ms/winget-settings", "CDN"}; + return Experiment{ "winget source CDN experiment", "CDN", "https://aka.ms/winget-settings", "CDN"}; #ifndef AICLI_DISABLE_TEST_HOOKS case Key::TestExperiment: return Experiment{ "Test experiment", "TestExperiment", "https://aka.ms/winget-settings", "TestExperiment" }; diff --git a/src/AppInstallerCommonCore/Public/winget/Experiment.h b/src/AppInstallerCommonCore/Public/winget/Experiment.h index 3678b3fbee..3d9a805e65 100644 --- a/src/AppInstallerCommonCore/Public/winget/Experiment.h +++ b/src/AppInstallerCommonCore/Public/winget/Experiment.h @@ -41,23 +41,23 @@ namespace AppInstaller::Settings using Key_t = std::underlying_type_t; - Experiment(std::string_view name, std::string_view jsonName, std::string_view link, std::string key) : - m_name(name), m_jsonName(jsonName), m_link(link), m_key(key) {} + Experiment(std::string name, std::string jsonName, std::string link, std::string key) : + m_name(std::move(name)), m_jsonName(jsonName), m_link(std::move(link)), m_key(std::move((key))) {} static ExperimentState GetState(Key feature); static ExperimentState GetStateInternal(Key feature); static Experiment GetExperiment(Key key); static std::vector GetAllExperiments(); - std::string_view Name() const { return m_name; } + std::string Name() const { return m_name; } Utility::LocIndView JsonName() const { return m_jsonName; } - std::string_view Link() const { return m_link; } + std::string Link() const { return m_link; } std::string GetKey() const { return m_key; } private: - std::string_view m_name; + std::string m_name; Utility::LocIndView m_jsonName; - std::string_view m_link; + std::string m_link; std::string m_key; }; } diff --git a/src/AppInstallerSharedLib/JsonUtil.cpp b/src/AppInstallerSharedLib/JsonUtil.cpp index 4373c6de96..221eccba6d 100644 --- a/src/AppInstallerSharedLib/JsonUtil.cpp +++ b/src/AppInstallerSharedLib/JsonUtil.cpp @@ -77,9 +77,10 @@ namespace AppInstaller::JSON { for (const auto& entry : node.getMemberNames()) { - if (node[entry].isBool()) + auto& value = node[entry]; + if (value.isBool()) { - result[entry] = node[entry].asBool(); + result[entry] = value.asBool(); } } diff --git a/src/AppInstallerSharedLib/Public/winget/JsonUtil.h b/src/AppInstallerSharedLib/Public/winget/JsonUtil.h index 5d77ef0e2f..559252c223 100644 --- a/src/AppInstallerSharedLib/Public/winget/JsonUtil.h +++ b/src/AppInstallerSharedLib/Public/winget/JsonUtil.h @@ -35,7 +35,7 @@ namespace AppInstaller::JSON std::optional> GetValue>(const Json::Value& node); template<> - std::optional> GetValue(const Json::Value& node); + std::optional> GetValue>(const Json::Value& node); #ifndef WINGET_DISABLE_FOR_FUZZING // For cpprestsdk JSON