Skip to content
This repository has been archived by the owner on Sep 8, 2020. It is now read-only.

syntax refactor #477

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

syntax refactor #477

wants to merge 4 commits into from

Conversation

wuliupo
Copy link
Contributor

@wuliupo wuliupo commented Jul 20, 2016

Only syntax refactor,

  1. remove unused return null;
  2. a in b write in b.a will more clear;
  3. wrappers only have one property helper;
  4. _restore and _destroy were simplified as delete item.sortable.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.6%) to 99.383% when pulling b68ae26 on wuliupo:master into 4bb4493 on angular-ui:master.

src/sortable.js Outdated
@@ -20,119 +20,114 @@ angular.module('ui.sortable', [])
uiSortable: '='
},
link: function(scope, element, attrs, ngModel) {
'use strict';
Copy link
Contributor

@thgreasi thgreasi Jul 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm totally in favor of 'use strict', but this might cause errors in some ASP.Net environments.
jQuery prior v3 also didn't use 'use strict' for exactly that reason.

Copy link
Contributor Author

@wuliupo wuliupo Jul 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I remove this line! Thanks!

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.6%) to 99.383% when pulling 8e728b4 on wuliupo:master into 4bb4493 on angular-ui:master.


// update changed options
angular.forEach(newVal, function(value, key) {
// if it's a custom option of the directive,
// handle it approprietly
if (key in directiveOpts) {
if (directiveOpts[key]) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not equivalent in case of directiveOpts members with falsy values.

@thgreasi
Copy link
Contributor

Please don't create a hure diff. Try not moving methods until the initial refactor gets reviewed.
Keep in mind that targeting the absolute min number of code lines can hurt the debug-ability of the code. Also refactoring chained method invocations to be in the same line can hurt the readability of the code.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants