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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix repository path resolution #503

Merged
merged 2 commits into from Jul 22, 2016
Merged

Conversation

danilopopeye
Copy link
Contributor

There was missing the option for when using groups and defining the repository as a String (like, for default group).

This should (hopefully) fix some issues, like #473, #497, #334 馃槃

Danilo Sousa added 2 commits July 20, 2016 06:57
this correctly respects the repo declaration as Hash or String
apart from the refactorings, now the node path its correct even
when groups are used
@ytti
Copy link
Owner

ytti commented Jul 22, 2016

Thanks. Seems like non-trivial amount of work. Bit worried about removal of locking, but can't justify the need for it either.

@ytti ytti merged commit dd6f5df into ytti:master Jul 22, 2016
@danilopopeye
Copy link
Contributor Author

Bit worried about removal of locking, but can't justify the need for it either.

I haven't removed it, still here 馃憠 lib/oxidized/nodes.rb#L169-L176 馃槈

@danilopopeye danilopopeye deleted the ds-fixes-and-refactors branch July 22, 2016 13:00
@ytti
Copy link
Owner

ytti commented Jul 22, 2016

Aah my bad. I don't really know if we actually even need it, I'm not sure. I guess better safe than sorry.

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

Successfully merging this pull request may close these issues.

None yet

2 participants