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

Let #load_site not set site ivar #1213

Merged
merged 1 commit into from Sep 3, 2017
Merged

Let #load_site not set site ivar #1213

merged 1 commit into from Sep 3, 2017

Conversation

@ddfreyne
Copy link
Member

@ddfreyne ddfreyne commented Sep 3, 2017

The mutable state introduced by letting #load_site set the @site ivar is troubling (see #1211 for a case where I was bitten by this).

This PR lets #load_site return the new site, rather than set @site. I’ve opted to let the command runners assign the return value of #load_site to @site, which, even though not perfect, is more reasonable.

@ddfreyne ddfreyne force-pushed the immutable-load-site branch from d59e26e to 1ac16ba Sep 3, 2017
@ddfreyne ddfreyne merged commit d36cb4a into master Sep 3, 2017
3 checks passed
Loading
@ddfreyne ddfreyne deleted the immutable-load-site branch Sep 3, 2017
@ddfreyne ddfreyne mentioned this pull request Sep 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

1 participant