You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by Nitin Galave <ni...@gmail.com> on 2018/01/25 05:16:54 UTC
Re: Review Request 62532: RANGER-1805: Code improvement to follow best
practices in js
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62532/
-----------------------------------------------------------
(Updated Jan. 25, 2018, 5:16 a.m.)
Review request for ranger, Gautam Borad, Mehul Parikh, and Velmurugan Periasamy.
Changes
-------
Updated patch to resolved conflict with latest code base.
Bugs: RANGER-1805
https://issues.apache.org/jira/browse/RANGER-1805
Repository: ranger
Description
-------
Javascript coding best practices.
Diffs (updated)
-----
security-admin/src/main/webapp/scripts/modules/XAOverrides.js 82a84da
security-admin/src/main/webapp/scripts/modules/globalize/message/en.js 2cd3345
security-admin/src/main/webapp/scripts/utils/XAEnums.js 164e939
security-admin/src/main/webapp/scripts/utils/XATemplateHelpers.js 785e2c2
security-admin/src/main/webapp/scripts/views/DownloadServicePolicy.js 1b1a4aa
security-admin/src/main/webapp/scripts/views/common/AddGroup.js 81fd901
security-admin/src/main/webapp/scripts/views/policies/PermissionList.js 73d5417
security-admin/src/main/webapp/scripts/views/policymanager/ServiceLayout.js ab42b5c
security-admin/src/main/webapp/scripts/views/reports/AuditLayout.js 0e864b6
security-admin/src/main/webapp/scripts/views/reports/UserAccessLayout.js c5dc053
security-admin/src/main/webapp/scripts/views/user/UserProfileForm.js a385592
security-admin/src/main/webapp/templates/common/Footer_tmpl.html e3accb6
security-admin/src/main/webapp/templates/common/downloadservicepolicy_tmpl.html 49be577
security-admin/src/main/webapp/templates/helpers/XAHelpers.js 9363c6b
security-admin/src/main/webapp/templates/kms/KmsKeyCreate_tmpl.html 2aaac43
security-admin/src/main/webapp/templates/policies/RangerPolicyCreate_tmpl.html e5e4ce7
security-admin/src/main/webapp/templates/service/ServiceCreate_tmpl.html d79028e
security-admin/src/main/webapp/templates/users/GroupCreate_tmpl.html c387e68
security-admin/src/main/webapp/templates/users/UserCreate_tmpl.html 204e832
Diff: https://reviews.apache.org/r/62532/diff/2/
Changes: https://reviews.apache.org/r/62532/diff/1-2/
Testing
-------
Done overall basic UI testing which includes service, policy, user, group and permission modules.
Thanks,
Nitin Galave
Re: Review Request 62532: RANGER-1805: Code improvement to follow best
practices in js
Posted by Nitin Galave <ni...@gmail.com>.
> On Jan. 31, 2018, 7:28 a.m., Qiang Zhang wrote:
> > security-admin/src/main/webapp/scripts/views/DownloadServicePolicy.js
> > Line 45 (original)
> > <https://reviews.apache.org/r/62532/diff/2/?file=1946497#file1946497line45>
> >
> > Have you tested if it affects functionality?
This is assingned value to ui object that is never used.
Have you tested if it affects functionality?
<<Yes, I have tested this patch and working as expected.
> On Jan. 31, 2018, 7:28 a.m., Qiang Zhang wrote:
> > security-admin/src/main/webapp/scripts/views/common/AddGroup.js
> > Lines 96-97 (original)
> > <https://reviews.apache.org/r/62532/diff/2/?file=1946498#file1946498line96>
> >
> > Are you sure you want to delete them?multi records input will not be supported after deleting it.
display function parameter value will always be array even if I add user to single/multiple group.
> On Jan. 31, 2018, 7:28 a.m., Qiang Zhang wrote:
> > security-admin/src/main/webapp/scripts/views/policies/PermissionList.js
> > Lines 294-297 (original)
> > <https://reviews.apache.org/r/62532/diff/2/?file=1946499#file1946499line294>
> >
> > *
Unused local varible.
> On Jan. 31, 2018, 7:28 a.m., Qiang Zhang wrote:
> > security-admin/src/main/webapp/scripts/views/policymanager/ServiceLayout.js
> > Line 131 (original), 131 (patched)
> > <https://reviews.apache.org/r/62532/diff/2/?file=1946500#file1946500line131>
> >
> > *
Redundant checks.
> On Jan. 31, 2018, 7:28 a.m., Qiang Zhang wrote:
> > security-admin/src/main/webapp/scripts/views/reports/UserAccessLayout.js
> > Line 313 (original)
> > <https://reviews.apache.org/r/62532/diff/2/?file=1946502#file1946502line313>
> >
> > *
Duplicate object key
> On Jan. 31, 2018, 7:28 a.m., Qiang Zhang wrote:
> > security-admin/src/main/webapp/templates/common/downloadservicepolicy_tmpl.html
> > Line 28 (original), 28 (patched)
> > <https://reviews.apache.org/r/62532/diff/2/?file=1946505#file1946505line28>
> >
> > *
Unwanted attribute.
- Nitin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62532/#review196555
-----------------------------------------------------------
On Jan. 25, 2018, 5:16 a.m., Nitin Galave wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62532/
> -----------------------------------------------------------
>
> (Updated Jan. 25, 2018, 5:16 a.m.)
>
>
> Review request for ranger, Gautam Borad, Mehul Parikh, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-1805
> https://issues.apache.org/jira/browse/RANGER-1805
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Javascript coding best practices.
>
>
> Diffs
> -----
>
> security-admin/src/main/webapp/scripts/modules/XAOverrides.js 82a84da
> security-admin/src/main/webapp/scripts/modules/globalize/message/en.js 2cd3345
> security-admin/src/main/webapp/scripts/utils/XAEnums.js 164e939
> security-admin/src/main/webapp/scripts/utils/XATemplateHelpers.js 785e2c2
> security-admin/src/main/webapp/scripts/views/DownloadServicePolicy.js 1b1a4aa
> security-admin/src/main/webapp/scripts/views/common/AddGroup.js 81fd901
> security-admin/src/main/webapp/scripts/views/policies/PermissionList.js 73d5417
> security-admin/src/main/webapp/scripts/views/policymanager/ServiceLayout.js ab42b5c
> security-admin/src/main/webapp/scripts/views/reports/AuditLayout.js 0e864b6
> security-admin/src/main/webapp/scripts/views/reports/UserAccessLayout.js c5dc053
> security-admin/src/main/webapp/scripts/views/user/UserProfileForm.js a385592
> security-admin/src/main/webapp/templates/common/Footer_tmpl.html e3accb6
> security-admin/src/main/webapp/templates/common/downloadservicepolicy_tmpl.html 49be577
> security-admin/src/main/webapp/templates/helpers/XAHelpers.js 9363c6b
> security-admin/src/main/webapp/templates/kms/KmsKeyCreate_tmpl.html 2aaac43
> security-admin/src/main/webapp/templates/policies/RangerPolicyCreate_tmpl.html e5e4ce7
> security-admin/src/main/webapp/templates/service/ServiceCreate_tmpl.html d79028e
> security-admin/src/main/webapp/templates/users/GroupCreate_tmpl.html c387e68
> security-admin/src/main/webapp/templates/users/UserCreate_tmpl.html 204e832
>
>
> Diff: https://reviews.apache.org/r/62532/diff/2/
>
>
> Testing
> -------
>
> Done overall basic UI testing which includes service, policy, user, group and permission modules.
>
>
> Thanks,
>
> Nitin Galave
>
>
Re: Review Request 62532: RANGER-1805: Code improvement to follow best
practices in js
Posted by Qiang Zhang <zh...@zte.com.cn>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62532/#review196555
-----------------------------------------------------------
Some changes will affect the function, you have a comprehensive exception tests?
security-admin/src/main/webapp/scripts/views/DownloadServicePolicy.js
Line 45 (original)
<https://reviews.apache.org/r/62532/#comment276246>
Have you tested if it affects functionality?
security-admin/src/main/webapp/scripts/views/common/AddGroup.js
Lines 96-97 (original)
<https://reviews.apache.org/r/62532/#comment276247>
Are you sure you want to delete them?multi records input will not be supported after deleting it.
security-admin/src/main/webapp/scripts/views/policies/PermissionList.js
Lines 294-297 (original)
<https://reviews.apache.org/r/62532/#comment276248>
*
security-admin/src/main/webapp/scripts/views/policies/PermissionList.js
Lines 469-473 (original)
<https://reviews.apache.org/r/62532/#comment276249>
*
security-admin/src/main/webapp/scripts/views/policymanager/ServiceLayout.js
Line 131 (original), 131 (patched)
<https://reviews.apache.org/r/62532/#comment276250>
*
security-admin/src/main/webapp/scripts/views/policymanager/ServiceLayout.js
Line 179 (original), 179 (patched)
<https://reviews.apache.org/r/62532/#comment276251>
*
security-admin/src/main/webapp/scripts/views/reports/UserAccessLayout.js
Line 313 (original)
<https://reviews.apache.org/r/62532/#comment276252>
*
security-admin/src/main/webapp/scripts/views/reports/UserAccessLayout.js
Line 349 (original)
<https://reviews.apache.org/r/62532/#comment276253>
*
security-admin/src/main/webapp/scripts/views/user/UserProfileForm.js
Line 140 (original), 140 (patched)
<https://reviews.apache.org/r/62532/#comment276254>
*
security-admin/src/main/webapp/templates/common/downloadservicepolicy_tmpl.html
Line 28 (original), 28 (patched)
<https://reviews.apache.org/r/62532/#comment276255>
*
- Qiang Zhang
On 一月 25, 2018, 5:16 a.m., Nitin Galave wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62532/
> -----------------------------------------------------------
>
> (Updated 一月 25, 2018, 5:16 a.m.)
>
>
> Review request for ranger, Gautam Borad, Mehul Parikh, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-1805
> https://issues.apache.org/jira/browse/RANGER-1805
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Javascript coding best practices.
>
>
> Diffs
> -----
>
> security-admin/src/main/webapp/scripts/modules/XAOverrides.js 82a84da
> security-admin/src/main/webapp/scripts/modules/globalize/message/en.js 2cd3345
> security-admin/src/main/webapp/scripts/utils/XAEnums.js 164e939
> security-admin/src/main/webapp/scripts/utils/XATemplateHelpers.js 785e2c2
> security-admin/src/main/webapp/scripts/views/DownloadServicePolicy.js 1b1a4aa
> security-admin/src/main/webapp/scripts/views/common/AddGroup.js 81fd901
> security-admin/src/main/webapp/scripts/views/policies/PermissionList.js 73d5417
> security-admin/src/main/webapp/scripts/views/policymanager/ServiceLayout.js ab42b5c
> security-admin/src/main/webapp/scripts/views/reports/AuditLayout.js 0e864b6
> security-admin/src/main/webapp/scripts/views/reports/UserAccessLayout.js c5dc053
> security-admin/src/main/webapp/scripts/views/user/UserProfileForm.js a385592
> security-admin/src/main/webapp/templates/common/Footer_tmpl.html e3accb6
> security-admin/src/main/webapp/templates/common/downloadservicepolicy_tmpl.html 49be577
> security-admin/src/main/webapp/templates/helpers/XAHelpers.js 9363c6b
> security-admin/src/main/webapp/templates/kms/KmsKeyCreate_tmpl.html 2aaac43
> security-admin/src/main/webapp/templates/policies/RangerPolicyCreate_tmpl.html e5e4ce7
> security-admin/src/main/webapp/templates/service/ServiceCreate_tmpl.html d79028e
> security-admin/src/main/webapp/templates/users/GroupCreate_tmpl.html c387e68
> security-admin/src/main/webapp/templates/users/UserCreate_tmpl.html 204e832
>
>
> Diff: https://reviews.apache.org/r/62532/diff/2/
>
>
> Testing
> -------
>
> Done overall basic UI testing which includes service, policy, user, group and permission modules.
>
>
> Thanks,
>
> Nitin Galave
>
>
Re: Review Request 62532: RANGER-1805: Code improvement to follow best
practices in js
Posted by Qiang Zhang <zh...@zte.com.cn>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62532/#review196558
-----------------------------------------------------------
Ship it!
Ship It!
- Qiang Zhang
On 一月 25, 2018, 5:16 a.m., Nitin Galave wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62532/
> -----------------------------------------------------------
>
> (Updated 一月 25, 2018, 5:16 a.m.)
>
>
> Review request for ranger, Gautam Borad, Mehul Parikh, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-1805
> https://issues.apache.org/jira/browse/RANGER-1805
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Javascript coding best practices.
>
>
> Diffs
> -----
>
> security-admin/src/main/webapp/scripts/modules/XAOverrides.js 82a84da
> security-admin/src/main/webapp/scripts/modules/globalize/message/en.js 2cd3345
> security-admin/src/main/webapp/scripts/utils/XAEnums.js 164e939
> security-admin/src/main/webapp/scripts/utils/XATemplateHelpers.js 785e2c2
> security-admin/src/main/webapp/scripts/views/DownloadServicePolicy.js 1b1a4aa
> security-admin/src/main/webapp/scripts/views/common/AddGroup.js 81fd901
> security-admin/src/main/webapp/scripts/views/policies/PermissionList.js 73d5417
> security-admin/src/main/webapp/scripts/views/policymanager/ServiceLayout.js ab42b5c
> security-admin/src/main/webapp/scripts/views/reports/AuditLayout.js 0e864b6
> security-admin/src/main/webapp/scripts/views/reports/UserAccessLayout.js c5dc053
> security-admin/src/main/webapp/scripts/views/user/UserProfileForm.js a385592
> security-admin/src/main/webapp/templates/common/Footer_tmpl.html e3accb6
> security-admin/src/main/webapp/templates/common/downloadservicepolicy_tmpl.html 49be577
> security-admin/src/main/webapp/templates/helpers/XAHelpers.js 9363c6b
> security-admin/src/main/webapp/templates/kms/KmsKeyCreate_tmpl.html 2aaac43
> security-admin/src/main/webapp/templates/policies/RangerPolicyCreate_tmpl.html e5e4ce7
> security-admin/src/main/webapp/templates/service/ServiceCreate_tmpl.html d79028e
> security-admin/src/main/webapp/templates/users/GroupCreate_tmpl.html c387e68
> security-admin/src/main/webapp/templates/users/UserCreate_tmpl.html 204e832
>
>
> Diff: https://reviews.apache.org/r/62532/diff/2/
>
>
> Testing
> -------
>
> Done overall basic UI testing which includes service, policy, user, group and permission modules.
>
>
> Thanks,
>
> Nitin Galave
>
>
Re: Review Request 62532: RANGER-1805: Code improvement to follow best
practices in js
Posted by Zsombor Gegesy <gz...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62532/#review196232
-----------------------------------------------------------
Ship it!
- Zsombor Gegesy
On Jan. 25, 2018, 5:16 a.m., Nitin Galave wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62532/
> -----------------------------------------------------------
>
> (Updated Jan. 25, 2018, 5:16 a.m.)
>
>
> Review request for ranger, Gautam Borad, Mehul Parikh, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-1805
> https://issues.apache.org/jira/browse/RANGER-1805
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Javascript coding best practices.
>
>
> Diffs
> -----
>
> security-admin/src/main/webapp/scripts/modules/XAOverrides.js 82a84da
> security-admin/src/main/webapp/scripts/modules/globalize/message/en.js 2cd3345
> security-admin/src/main/webapp/scripts/utils/XAEnums.js 164e939
> security-admin/src/main/webapp/scripts/utils/XATemplateHelpers.js 785e2c2
> security-admin/src/main/webapp/scripts/views/DownloadServicePolicy.js 1b1a4aa
> security-admin/src/main/webapp/scripts/views/common/AddGroup.js 81fd901
> security-admin/src/main/webapp/scripts/views/policies/PermissionList.js 73d5417
> security-admin/src/main/webapp/scripts/views/policymanager/ServiceLayout.js ab42b5c
> security-admin/src/main/webapp/scripts/views/reports/AuditLayout.js 0e864b6
> security-admin/src/main/webapp/scripts/views/reports/UserAccessLayout.js c5dc053
> security-admin/src/main/webapp/scripts/views/user/UserProfileForm.js a385592
> security-admin/src/main/webapp/templates/common/Footer_tmpl.html e3accb6
> security-admin/src/main/webapp/templates/common/downloadservicepolicy_tmpl.html 49be577
> security-admin/src/main/webapp/templates/helpers/XAHelpers.js 9363c6b
> security-admin/src/main/webapp/templates/kms/KmsKeyCreate_tmpl.html 2aaac43
> security-admin/src/main/webapp/templates/policies/RangerPolicyCreate_tmpl.html e5e4ce7
> security-admin/src/main/webapp/templates/service/ServiceCreate_tmpl.html d79028e
> security-admin/src/main/webapp/templates/users/GroupCreate_tmpl.html c387e68
> security-admin/src/main/webapp/templates/users/UserCreate_tmpl.html 204e832
>
>
> Diff: https://reviews.apache.org/r/62532/diff/2/
>
>
> Testing
> -------
>
> Done overall basic UI testing which includes service, policy, user, group and permission modules.
>
>
> Thanks,
>
> Nitin Galave
>
>
Re: Review Request 62532: RANGER-1805: Code improvement to follow best
practices in js
Posted by Pradeep Agrawal <pr...@freestoneinfotech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62532/#review196224
-----------------------------------------------------------
Ship it!
Ship It!
- Pradeep Agrawal
On Jan. 25, 2018, 5:16 a.m., Nitin Galave wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62532/
> -----------------------------------------------------------
>
> (Updated Jan. 25, 2018, 5:16 a.m.)
>
>
> Review request for ranger, Gautam Borad, Mehul Parikh, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-1805
> https://issues.apache.org/jira/browse/RANGER-1805
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Javascript coding best practices.
>
>
> Diffs
> -----
>
> security-admin/src/main/webapp/scripts/modules/XAOverrides.js 82a84da
> security-admin/src/main/webapp/scripts/modules/globalize/message/en.js 2cd3345
> security-admin/src/main/webapp/scripts/utils/XAEnums.js 164e939
> security-admin/src/main/webapp/scripts/utils/XATemplateHelpers.js 785e2c2
> security-admin/src/main/webapp/scripts/views/DownloadServicePolicy.js 1b1a4aa
> security-admin/src/main/webapp/scripts/views/common/AddGroup.js 81fd901
> security-admin/src/main/webapp/scripts/views/policies/PermissionList.js 73d5417
> security-admin/src/main/webapp/scripts/views/policymanager/ServiceLayout.js ab42b5c
> security-admin/src/main/webapp/scripts/views/reports/AuditLayout.js 0e864b6
> security-admin/src/main/webapp/scripts/views/reports/UserAccessLayout.js c5dc053
> security-admin/src/main/webapp/scripts/views/user/UserProfileForm.js a385592
> security-admin/src/main/webapp/templates/common/Footer_tmpl.html e3accb6
> security-admin/src/main/webapp/templates/common/downloadservicepolicy_tmpl.html 49be577
> security-admin/src/main/webapp/templates/helpers/XAHelpers.js 9363c6b
> security-admin/src/main/webapp/templates/kms/KmsKeyCreate_tmpl.html 2aaac43
> security-admin/src/main/webapp/templates/policies/RangerPolicyCreate_tmpl.html e5e4ce7
> security-admin/src/main/webapp/templates/service/ServiceCreate_tmpl.html d79028e
> security-admin/src/main/webapp/templates/users/GroupCreate_tmpl.html c387e68
> security-admin/src/main/webapp/templates/users/UserCreate_tmpl.html 204e832
>
>
> Diff: https://reviews.apache.org/r/62532/diff/2/
>
>
> Testing
> -------
>
> Done overall basic UI testing which includes service, policy, user, group and permission modules.
>
>
> Thanks,
>
> Nitin Galave
>
>