Closed sumitzarkar closed 4 years ago
Thanks for the hard work on this. @oscarvilla1 and I tested yesterday and didn't notice and issues.
I noticed a few minor remaining unused variables that I will probably do a PR for into your branch.
Speaking of PRs, I should discuss with the team where (which branch) the changes you made should go (they may not get merged into dev, but another long-lived branch - like https://github.com/Esri/distance-direction-addin-dotnet/tree/dev-Pro2.6 ) - so we will leave this PR open until we make a decision.
But I did want to let you know that things look and test as expected and nice job. Thanks
Awesome, thank you
From: Chris Moore notifications@github.com Sent: Wednesday, February 12, 2020 5:54 AM To: Esri/distance-direction-addin-dotnet distance-direction-addin-dotnet@noreply.github.com Cc: Oscar Villa ovilla@esri.com; Mention mention@noreply.github.com Subject: Re: [Esri/distance-direction-addin-dotnet] Delivery for Distance And Direction Addin - #684 (#685)
Thanks for the hard work on this. @OscarVilla1https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_OscarVilla1&d=DwMCaQ&c=n6-cguzQvX_tUIrZOS_4Og&r=_KhdAIs_BZjuHsKoZgk9N2knCgjme3-JIdfsykKOwDo&m=Y38gADQKbe8atp1dRMU8Fxx7wEwJncFUva5LV1PdWYs&s=gX_6q0GJ8bzwjmGGEjxy1XGCN_L7x8ECDYkW_Ft8do0&e= and I tested yesterday and didn't notice and issues.
I noticed a few minor remaining unused variables that I will probably do a PR for into your branch.
Speaking of PRs, I should discuss with the team where (which branch) the changes you made should go (they may not get merged into dev, but another long-lived branch - like https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Esri_distance-2Ddirection-2Daddin-2Ddotnet_tree_dev-2DPro2.6&d=DwIF-g&c=n6-cguzQvX_tUIrZOS_4Og&r=UmA4la6omLDA-zIyopOQB5SngJNjAn5HEi-QaiWlmQANed_7oT_aC88Rq5KqHd-c_m_PwWg8Jo1vnBGIV0FldsEZNNWYD0veTeS-V6UzDIo&m=lGDyEKy0VFuV1YpGQZwG1_fXsnAVvx84GytPCP0bsrY&s=d_eilyaDmalN3KVbp45bNJWsCC8xr5Qmo35C6RQ78kQ&e= https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Esri_distance-2Ddirection-2Daddin-2Ddotnet_tree_dev-2DPro2.6&d=DwMCaQ&c=n6-cguzQvX_tUIrZOS_4Og&r=_KhdAIs_BZjuHsKoZgk9N2knCgjme3-JIdfsykKOwDo&m=Y38gADQKbe8atp1dRMU8Fxx7wEwJncFUva5LV1PdWYs&s=uliesaUppyWzHJL7uWVR2LvhbbcjKc4MIaTbUvEZiTI&e= ) - so we will leave this PR open until we make a decision.
But I did want to let you know that things look and test as expected and nice job. Thanks
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Esri_distance-2Ddirection-2Daddin-2Ddotnet_pull_685-3Femail-5Fsource-3Dnotifications-26email-5Ftoken-3DAOK7XTOKWOLTMJ5P5XKRMVDRCP5QXA5CNFSM4KTF64MKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELQ23ZI-23issuecomment-2D585215461&d=DwMCaQ&c=n6-cguzQvX_tUIrZOS_4Og&r=_KhdAIs_BZjuHsKoZgk9N2knCgjme3-JIdfsykKOwDo&m=Y38gADQKbe8atp1dRMU8Fxx7wEwJncFUva5LV1PdWYs&s=iZQdUS2krAXRzst_LQdKQzOm0ndjm1V58qrMcq2NLD4&e=, or unsubscribehttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AOK7XTJWJTNXDMBUYW624RTRCP5QXANCNFSM4KTF64MA&d=DwMCaQ&c=n6-cguzQvX_tUIrZOS_4Og&r=_KhdAIs_BZjuHsKoZgk9N2knCgjme3-JIdfsykKOwDo&m=Y38gADQKbe8atp1dRMU8Fxx7wEwJncFUva5LV1PdWYs&s=I1V4R9201w3LReV7W_GAyio4WyTxitnFRxgPvpMeSXk&e=.
Opened alternate PR #686 - so we could just switch the head branch we are merging into (branch: Pro2.6 vs. dev). Feel free to move any discussion to that PR and sorry for any confusion.
Closing unmerged to merge into different head branch in PR #686
Implemented following points:
In Addition to this, we have also commited the code for UAT feedback mentioned in https://github.com/Esri/distance-direction-addin-dotnet/issues/684#issuecomment-584437025