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

updated unifi-linuxserver #192

Merged
merged 4 commits into from
Oct 14, 2019
Merged

updated unifi-linuxserver #192

merged 4 commits into from
Oct 14, 2019

Conversation

magicalyak
Copy link
Contributor

Updated repo to point to new linuxserver unfi-controller repo since the unifi repo is depracted. The version is already out of support. The new repo is under support and has no impact on an existing installation. It is also recommended by unifi as the preferred docker method. We also have an option for an extra argument to cap memory but I put that in the more_info section rather than forcing the user to enter it. This is for #191

@phillxnet phillxnet added the needs review Test install, function, on / off behaviour, all links / info. label Aug 31, 2019
@phillxnet
Copy link
Member

@magicalyak Appologies for slow response. Another fine contribution here, thanks. I'd love to merge this but given I know nothing about it I'd rather have another set of eyes on it quickly before we merge and publish.

Adding a needs review and if all looks good we can get this out.

Thanks again @magicalyak I'm sure this will be much appreciated.

But given you are one of our main rock-on reviewers and this is a fairly trivial change buzz here again if we get no review takers and we can take it from there.

A quick note however: we are trying to surface both the upstream project, via the website entry (Rock-on title) and the docker image used. The latter we are some what awkwardly embedding in the description currently. So this pr might be a good time to give this rock-on that particular face lift. Then we honour the upstream project and surface the particular docker image at the same time.

The few examples we currently have of this direction are the following:

Pi-Hole which you and @FroggyFlox recently worked on.
and
Seafile
and
Teamspeak
So if you fancy upping this one to match them that would be great. Just trying to nudge all new Rock-ons commits to conform to this pattern. We need a nicer internal system really but at least this way we present better and honour the upstream projects with the main link as the docker links are often more of lower level technical interest.

Hope your game and thanks again for getting this older image problem sorted.

@phillxnet
Copy link
Member

@magicalyak Linking for context to our related issue re sorting a 'proper' system to surface both the upstream project and the docker image used:

Make Rock-on links consistent #31

@magicalyak
Copy link
Contributor Author

Does that work? @phillxnet

Copy link
Member

@FroggyFlox FroggyFlox left a comment

Choose a reason for hiding this comment

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

Thanks a lot for catching this @magicalyak !
I can confirm it works as intended: installs, toggles ON/OFF, and uninstalls without error. The webUI is also accessible without problem.

I took the liberty to suggest the changes @phillxnet referred to.

Thanks again!

unifi-linuxserver.json Outdated Show resolved Hide resolved
unifi-linuxserver.json Outdated Show resolved Hide resolved
@mikey0000
Copy link
Contributor

I'm guessing this will default to the latest tag, from what I can see it is correct (from my review) for what thats worth.

@phillxnet
Copy link
Member

@mikey0000 A belated thanks for your input and yes all Rock-ons, if no tag is specified, will default to the latest.
@magicalyak Do you fancy committing @FroggyFlox suggestions and then we can get this one sorted at last.
Cheers.

@phillxnet phillxnet removed the needs review Test install, function, on / off behaviour, all links / info. label Oct 8, 2019
@magicalyak
Copy link
Contributor Author

Yes will do @phillxnet

@magicalyak
Copy link
Contributor Author

@phillxnet @FroggyFlox suggestions committed, apologies for delay.

@phillxnet
Copy link
Member

@magicalyak and @FroggyFlox Thanks for getting this much needed update into shape.
Merging now and publishing soon their after.

Fixes #191

@phillxnet phillxnet merged commit 560c965 into rockstor:master Oct 14, 2019
@phillxnet
Copy link
Member

@mikey0000 Thanks again for taking a look at this proposed Rock-on. Should be out shortly.

@phillxnet
Copy link
Member

@magicalyak @FroggyFlox @mikey0000 This Rock-on update is now live.
Thanks.

@phillxnet phillxnet mentioned this pull request Oct 14, 2019
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.

4 participants