Addressing comments

microsoft · Dec 12, 2024 · dee65a9 · dee65a9
1 parent bdf8c90
commit dee65a9
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 17 deletions.
diff --git 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<Setting::Experiments>();
             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
@@ -41,23 +41,23 @@ namespace AppInstaller::Settings
 
         using Key_t = std::underlying_type_t<Key>;
 
-        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<Experiment> 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
@@ -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
@@ -35,7 +35,7 @@ namespace AppInstaller::JSON
     std::optional<std::vector<std::string>> GetValue<std::vector<std::string>>(const Json::Value& node);
 
     template<>
-    std::optional<std::map<std::string, bool>> GetValue(const Json::Value& node);
+    std::optional<std::map<std::string, bool>> GetValue<std::map<std::string, bool>>(const Json::Value& node);
 
 #ifndef WINGET_DISABLE_FOR_FUZZING
     // For cpprestsdk JSON