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

Remove the code related to starting vncserver. (#4005) #4028

Merged
merged 1 commit into from Nov 30, 2017

Conversation

Projects
None yet
2 participants
@varshavaradarajan
Copy link
Contributor

commented Nov 21, 2017

  • Users having the old bootstrapper will not be affected by this change.

#4005

@varshavaradarajan varshavaradarajan requested a review from ketan Nov 21, 2017

@ketan

ketan approved these changes Nov 21, 2017

Copy link
Member

left a comment

This looks good to me, I think we should recommend that users use xvfb-run or run vncserver as a service on its own, and somehow ensure that the DISPLAY can be accessed by the gocd agent.

@ketan

This comment has been minimized.

Copy link
Member

commented Nov 21, 2017

Also, missed this line.

@@ -119,13 +118,6 @@ else
PID_FILE="$AGENT_WORK_DIR/go-agent.pid"
fi

if [ "$VNC" == "Y" ]; then

This comment has been minimized.

Copy link
@ketan

ketan Nov 21, 2017

Member

Thinking out aloud, should we warn if this variable is set, perhaps with some like to a documentation about possible fallbacks?

This comment has been minimized.

Copy link
@varshavaradarajan

varshavaradarajan Nov 21, 2017

Author Contributor

Really? I thought a note in the release notes should be fine. Also, is this even a change that would require us to add a deprecation note given that only people who upgrade the bootstrapper would face the problem? I don't mind adding the warning though. What fallbacks? We should ask people to start the vncserver on their own, right?

This comment has been minimized.

Copy link
@ketan

ketan Nov 21, 2017

Member

What fallbacks? We should ask people to start the vncserver on their own, right?

That's what I meant as "fallback", not automated, but something users can use to replace this functionality.

This comment has been minimized.

Copy link
@ketan

ketan Nov 21, 2017

Member

I'm not suggesting that we do this, I'm just thinking if there are things we can possibly do to make it easy for users.

@varshavaradarajan varshavaradarajan force-pushed the varshavaradarajan:remove-vnc branch from 1cb11ea to a73dec1 Nov 22, 2017

Remove the code related to starting vncserver. (#4005)
* Users having the old bootstrapper will not be affected by this change.

@ketan ketan force-pushed the varshavaradarajan:remove-vnc branch from a73dec1 to cdf6722 Nov 30, 2017

@ketan ketan merged commit cdf6722 into gocd:master Nov 30, 2017

1 check passed

license/cla Contributor License Agreement is signed.
Details
@ketan

This comment has been minimized.

Copy link
Member

commented Nov 30, 2017

@varshavaradarajan — when you have time, can you take a look and see if the warning is good enough?

@ketan ketan added this to the Release 17.12 milestone Nov 30, 2017

@varshavaradarajan varshavaradarajan deleted the varshavaradarajan:remove-vnc branch Jan 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.