Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update position when scope changes #67

Merged
merged 1 commit into from
Jan 30, 2013
Merged

Update position when scope changes #67

merged 1 commit into from
Jan 30, 2013

Conversation

philippfranke
Copy link
Contributor

After discussion on #19, I tried to fix that issue.

@tedkulp
Copy link

tedkulp commented Jan 19, 2013

👍

This works great with my limited testing on a Greenfield app. The only thing I could think is that people might want this to be optional in order not to break backwards compatibility.

@philippfranke
Copy link
Contributor Author

@tedkulp, you're right about backwards compatibility. In a few days I'm going to write some tests and implement the ability to enable/disable this function.

/cc @swanandp

@narath
Copy link

narath commented Jan 27, 2013

👍
Good call on the default behaviour, just let it be the position, and remove / insert appropriately. Cool.

@philippfranke philippfranke reopened this Jan 30, 2013
swanandp added a commit that referenced this pull request Jan 30, 2013
Update position when scope changes
@swanandp swanandp merged commit 466feb5 into brendon:master Jan 30, 2013
@swanandp
Copy link
Contributor

I think this calls for a few more test cases, but looks good to me.

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

Successfully merging this pull request may close these issues.

None yet

4 participants