Implement the endpoint input function. #8

oyzh888 · 2017-05-26T09:40:02Z

No description provided.

msftclas · 2017-05-26T09:40:06Z

@oyzh888,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

huxuan · 2017-05-26T09:43:20Z

SampleUserControlLibrary/SampleUserControl.xaml.cs

@@ -98,11 +98,18 @@ public string SubscriptionKey
            set;
        }

+        public string SubscriptionEndPoint


endpoint is one single word, so let us change EndPoint to Endpoint instead. Also for other places.

huxuan · 2017-05-26T09:45:22Z

SampleUserControlLibrary/SubscriptionKeyPage.xaml

+                <TextBox VerticalAlignment="Stretch" Grid.Column="1"  Padding="2" Text="{Binding SubscriptionKey, Mode=TwoWay}" Margin="0,5,0,5" Grid.ColumnSpan="2"/>
+                <Label VerticalAlignment="Center" Grid.Row="1" Content="EndPoint:" Margin="0,2" Height="26"/>
+                <TextBox VerticalAlignment="Stretch" Grid.Row="1"  Grid.Column="1" Padding="2" Text="{Binding SubscriptionEndPoint, Mode=TwoWay}" Margin="0,5,0,5" Grid.ColumnSpan="2"/>
+                <Button Click="SaveKey_Click" Grid.Row="2" Margin="78,7,73,6" Padding="5, 0, 5, 0" VerticalAlignment="Center" Height="18" Content="Save Key" Grid.Column="1" />


The Key in the function name and the Save Key label are misleading, let us change the function name to SaveSettings_Click with label Save Settings. Also apply this to delete.

huxuan · 2017-05-26T09:49:11Z

SampleUserControlLibrary/SubscriptionKeyPage.xaml.cs

+        private readonly string _defaultSubscriptionKeyPromptMessage = "Paste your subscription key here firstly";
+        private readonly string _defaultSubscriptionEndPointPromptMessage = "Paste your EndPoint here to start";
+
+        private static string s_subscriptionKey, s_subscriptionEndPoint;


Split them to two lines.

huxuan · 2017-05-26T09:54:46Z

SampleUserControlLibrary/SubscriptionKeyPage.xaml.cs

@@ -158,11 +231,13 @@ private void SaveKey_Click(object sender, RoutedEventArgs e)
            try
            {
                SaveSubscriptionKeyToIsolatedStorage(SubscriptionKey);
-                MessageBox.Show("Subscription key is saved in your disk.\nYou do not need to paste the key next time.", "Subscription Key");
+                SaveSubscriptionEndPointToIsolatedStorage(SubscriptionEndPoint);
+                //MessageBox.Show(SubscriptionKey + '\n' + SubscriptionEndPoint);


Please remove this comment.

huxuan · 2017-05-26T10:02:58Z

SampleUserControlLibrary/SubscriptionKeyPage.xaml.cs

-                MessageBox.Show("Subscription key is saved in your disk.\nYou do not need to paste the key next time.", "Subscription Key");
+                SaveSubscriptionEndPointToIsolatedStorage(SubscriptionEndPoint);
+                //MessageBox.Show(SubscriptionKey + '\n' + SubscriptionEndPoint);
+                MessageBox.Show("Subscription key and endpoint is saved in your disk.\nYou do not need to paste the key next time.", "Subscription Key");


The previous title of the messagebox Subscription KeyPlease change to Settings.

huxuan · 2017-05-26T10:03:20Z

SampleUserControlLibrary/SubscriptionKeyPage.xaml.cs

            }
            catch (System.Exception exception)
            {
-                MessageBox.Show("Fail to save subscription key. Error message: " + exception.Message,
+                MessageBox.Show("Fail to save subscription & endpoint key. Error message: " + exception.Message,


Fail to save subscription key & endpoint.

related to endpoint

huxuan

Just finish the last piece changes and make your first pull request fire off!

huxuan · 2017-05-26T11:21:55Z

SampleUserControlLibrary/SubscriptionKeyPage.xaml.cs

+            {
+                try
+                {
+                    using (var iStreamForEndPoint = new IsolatedStorageFileStream(_isolatedStorageSubscriptionEndpointFileName, FileMode.Open, isoStore))


s/EndPoint/Endpoint/g

huxuan · 2017-05-26T11:22:10Z

SampleUserControlLibrary/SubscriptionKeyPage.xaml.cs

+                {
+                    using (var iStreamForEndPoint = new IsolatedStorageFileStream(_isolatedStorageSubscriptionEndpointFileName, FileMode.Open, isoStore))
+                    {
+                        using (var readerForEndPoint = new StreamReader(iStreamForEndPoint))


s/EndPoint/Endpoint/g

huxuan · 2017-05-26T11:23:22Z

SampleUserControlLibrary/SubscriptionKeyPage.xaml.cs

+                    {
+                        using (var readerForEndPoint = new StreamReader(iStreamForEndPoint))
+                        {
+                            subscriptionEndpoint = readerForEndPoint.ReadLine();


huxuan · 2017-05-26T13:09:03Z

SampleUserControlLibrary/SubscriptionKeyPage.xaml.cs

                SaveSubscriptionKeyToIsolatedStorage("");
-                MessageBox.Show("Subscription key is deleted from your disk.", "Subscription Key");
+                MessageBox.Show("Subscription setting is deleted from your disk.", "Subscription Key");


The title also needs change.

huxuan · 2017-05-26T13:09:42Z

SampleUserControlLibrary/SubscriptionKeyPage.xaml.cs

            }
            catch (System.Exception exception)
            {
-                MessageBox.Show("Fail to save subscription key. Error message: " + exception.Message,
+                MessageBox.Show("Fail to save subscription key & endpoint . Error message: " + exception.Message,


Please change the messagebox title here. Also for other places.

huxuan · 2017-05-31T02:46:46Z

SampleUserControlLibrary/SampleUserControl.xaml.cs

@@ -104,6 +104,11 @@ public string SubscriptionEndpoint
            set;
        }

+        public void set_subscriptionPage_defaultEndpoint(string msg)


Usually, C# function name follows PascalCase [1] rather than starting with lower case with underline as separator.

msg is obscure, change it to endpoint instead.

No default is necessary, details explained at other places.

Ordinary functions should locate after initialization function.

[1] https://en.wikipedia.org/wiki/PascalCase

1,2,4 are accepted.
"Default" is to differentiate the setter of endpoint and the default value of endpoint

huxuan · 2017-05-31T02:48:02Z

SampleUserControlLibrary/SubscriptionKeyPage.xaml.cs

@@ -49,7 +49,7 @@ public partial class SubscriptionKeyPage : Page, INotifyPropertyChanged
        private readonly string _isolatedStorageSubscriptionEndpointFileName = "SubscriptionEndpoint.txt";

        private readonly string _defaultSubscriptionKeyPromptMessage = "Paste your subscription key here firstly";
-        private readonly string _defaultSubscriptionEndpointPromptMessage = "Paste your endpoint here to start";
+        private string _defaultSubscriptionEndpointPromptMessage = "Paste your endpoint here to start";


We do not need to change the default endpoint setting. Just set the endpoint should be enough. Just make the default default.

huxuan · 2017-05-31T02:49:03Z

SampleUserControlLibrary/SubscriptionKeyPage.xaml.cs

+        public void setDefaultSubscriptionEndpoint(string msg){
+            _defaultSubscriptionEndpointPromptMessage = msg;
+            s_subscriptionEndpoint = msg;
+


Remove the redundant empty line. They should never exist in any circumstances.

huxuan · 2017-05-31T02:53:44Z

SampleUserControlLibrary/SubscriptionKeyPage.xaml.cs

+        /// Set a default endpoint when there is no legal endpoint value
+        /// </summary>
+        /// <param name="msg"></param>
+        public void setDefaultSubscriptionEndpoint(string msg){


Drop the default in function name and comments.

The variable name msg is obscure, let us make it endpoint instead.

Function name should follows PascalCase.

oyzh888 · 2017-06-01T08:48:13Z

SampleUserControlLibrary/SubscriptionKeyPage.xaml.cs

+        /// <param name="endpoint"></param>
+        public void SetSubscriptionEndpoint(string endpoint){
+            string subscriptionEndpoint = null;
+            subscriptionEndpoint = GetSubscriptionEndpointFromIsolatedStorage();


In fact, I've tried, but we cannot simply set the public string SubscriptionEndpoint, if I change the setter function, it throws a null pointer exception(may be caused by binding). It's a big problem and I use my own way to handle this.

huxuan · 2017-06-01T10:20:53Z

SampleUserControlLibrary/SubscriptionKeyPage.xaml.cs

-        /// Handles the Click event of the subscription key save button.
+        /// Saves the subscription endpoint to isolated storage.
+        /// </summary>
+        /// <param name="subscriptionEndpoint">The subscription key.</param>


Update the doc here.

huxuan · 2017-06-01T10:21:04Z

SampleUserControlLibrary/SubscriptionKeyPage.xaml.cs

+        /// <summary>
+        /// Set a default endpoint when there is no legal endpoint value
+        /// </summary>
+        /// <param name="endpoint"></param>


Fulfill the doc here.

…is no legal endpoint value

huxuan · 2017-06-01T23:12:24Z

SampleUserControlLibrary/SampleUserControl.xaml.cs

@@ -65,7 +65,7 @@ public partial class SampleScenarios : UserControl
        public static DependencyProperty SampleScenarioListProperty =
            DependencyProperty.Register("SampleScenarioList", typeof(Scenario[]), typeof(SampleScenarios));

-        private SubscriptionKeyPage _subscriptionPage;
+        private SubscriptionKeyPage _subscriptionPage ;


Please remove the redundant empty space here.

Pleas enter the commit message for your changes. Lines starting

Implement the endpoint input function.

9a00153

msftclas added the cla-required label May 26, 2017

huxuan suggested changes May 26, 2017

View reviewed changes

Changes to support the outpoint input as well revision on other code

4a46438

related to endpoint

huxuan suggested changes May 26, 2017

View reviewed changes

huxuan reviewed May 26, 2017

View reviewed changes

huxuan requested review from delfu and wangjun-microsoft May 26, 2017 15:26

huxuan mentioned this pull request May 26, 2017

Cognitive services URL should be configurable microsoft/Cognitive-Face-Windows#33

Closed

oyzh888 added 3 commits May 27, 2017 10:11

Changes to let code look more standarized

fa3109b

mall changes to let code be looked better

bee9a04

New interface to set the default endpoint

7318eb4

huxuan suggested changes May 31, 2017

View reviewed changes

oyzh888 added 2 commits May 31, 2017 11:20

Implement the default endpoint setting

61e557f

Add set functions to endpoint

3b94322

oyzh888 commented Jun 1, 2017

View reviewed changes

huxuan reviewed Jun 1, 2017

View reviewed changes

Offer the function to set endpoint and set default endpoint if there …

695063a

…is no legal endpoint value

huxuan suggested changes Jun 1, 2017

View reviewed changes

Endpoint support fix

66ba04f

Pleas enter the commit message for your changes. Lines starting

huxuan approved these changes Jun 2, 2017

View reviewed changes

huxuan merged commit 4320248 into microsoft:master Jun 2, 2017

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement the endpoint input function. #8

Implement the endpoint input function. #8

oyzh888 commented May 26, 2017

msftclas commented May 26, 2017

huxuan May 26, 2017

huxuan May 26, 2017

huxuan May 26, 2017

huxuan May 26, 2017

huxuan May 26, 2017

huxuan May 26, 2017

huxuan left a comment

huxuan May 26, 2017

huxuan May 26, 2017

huxuan May 26, 2017

huxuan May 26, 2017

huxuan May 26, 2017 •

edited

Loading

huxuan May 31, 2017 •

edited

Loading

oyzh888 May 31, 2017

huxuan May 31, 2017

huxuan May 31, 2017

huxuan May 31, 2017

oyzh888 Jun 1, 2017

huxuan Jun 1, 2017

huxuan Jun 1, 2017

huxuan Jun 1, 2017

Implement the endpoint input function. #8

Implement the endpoint input function. #8

Conversation

oyzh888 commented May 26, 2017

msftclas commented May 26, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huxuan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huxuan May 26, 2017 • edited Loading

Choose a reason for hiding this comment

huxuan May 31, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huxuan May 26, 2017 •

edited

Loading

huxuan May 31, 2017 •

edited

Loading