Make company configurable as far as possible #8

jokram · 2017-01-17T08:49:42Z

Tried to make the code independent from Autodesk. So it should only depend on enterprice.constants.

Replaced "adsk" with "$KIT_ID" where possible
Replaced "Autodesk" with "GitHub Enterprise"
Replaced "ADS_USER" with "GHE_USER"
Replaced "ADS_PASSWORD_OR_TOKEN" with "GHE_PASSWORD_OR_TOKEN"

Exceptions:

config.include: Here I've not yet found a way to make this dependent on enterprise.constants
Readme.md: For another company the Readme should be adapted, because it is delivered to end users

Important Notes:

I've had a merge conflict on my test branch where I've already included feature/configurable-protocol
So I've merged feature/configurable-protocol into this branch. Best is if you first check the other pull request Add GITHUB_PROTOCOL support (http or https) #7
I'm not yet able to test these changes, because there are other changes necessary before I can run it in my company environment:
- JIRA link is different
- Conventions for service account is different
- Currently we are using SSH
- Installation of LFS might fail due to proxy authentification
But I want to address these issues with separate pull requests

…e-company

…D_OR_TOKEN

larsxschneider · 2017-01-17T20:52:26Z

setup.sh

        else
-            echo -n "Please enter your Autodesk username and press [ENTER] (empty for \"$STORED_GITHUB_ENTERPRISE_ACCOUNT\"): "
+            echo -n "Please enter your GitHub Enterprise username and press [ENTER] (empty for \"$STORED_GITHUB_ENTERPRISE_ACCOUNT\"): "


Some of our engineers confuse GitHub Enterprise with github.com. Therefore I used the company name here. Maybe we should make this an enterprise.config setting?

larsxschneider · 2017-01-17T20:56:44Z

setup.sh

-            git --git-dir="$KIT_PATH/.git" --work-tree="$KIT_PATH" checkout --quiet -B adsk-setup && \
+            # [jokram] Why is the checkout of a new branch adsk-setup necessary? The reset --hard will overwrite it, or?
+            # Or is it just to mark that this was the previous version?
+            git --git-dir="$KIT_PATH/.git" --work-tree="$KIT_PATH" checkout --quiet -B $KIT_ID-setup && \
            git --git-dir="$KIT_PATH/.git" --work-tree="$KIT_PATH" reset --quiet --hard origin/$BRANCH && \


The checkout is necessary to make sure we are on the "adsk-setup" branch. If you develop enterprise-config then you might be on branch X. If you run it then it would hard reset X to the latest production state and you would "lose" your changes.

I think now I got it:
git reset --hard origin/production will modify the current branch pointer (which might be branch-X) to also point to origin/production. And by this the branch-X would have been modified. So git checkout -B adsk-setup is just to ensure that you have temp. branch which can be modified by the reset command.

But why not just: git checkout -B adsk-setup origin/production?

That call can fail if you have local uncommitted changes.

OK. Next try: git checkout -f -B adsk-setup origin/production
Documentation for -f says:

When switching branches, proceed even if the index or the working tree differs from HEAD. This is used to throw away local changes.

larsxschneider · 2017-01-18T10:17:03Z

enterprise.constants

@@ -9,6 +9,10 @@ KIT_ID='adsk'
 # Define your GitHub Enterprise server
 GITHUB_SERVER='git.yourcompany.com'

+
+# Add the protocol used to access GitHub server: http or https
+GITHUB_PROTOCOL='https'


I think we should follow your naming convention from GHE_USER etc. here and call this GHE_HTTP?

Plus, I would vote against using the term PROTOCOL because that could also mean "SSH"...

But then maybe also for GITHUB_SERVER => GHE_SERVER

larsxschneider · 2017-01-18T10:17:51Z

lib/setup_helpers.sh

+KIT_PATH=$(dirname "$0")
+. "$KIT_PATH/../enterprise.constants"
+
+PROTOCOL=$GITHUB_PROTOCOL


Why did you define PROTOCOL here?

It would not be necessary, but I was thinking that the helpers normaly are independent from GitHub, so I wanted to avoid to mention in the helper functions no GITHUB_* variable.
What would you prefer?

I agree with this approach if it is a local variable in a function. But here PROTOCOL as well as GITHUB_PROTOCOL are global variables and therefore I would prefer to only use the latter.

larsxschneider · 2017-01-24T12:18:12Z

@jokram : I haven't forgotten this PR. I am just super busy right now. This looks pretty good - I'll merge it ASAP. Are you blocked in any way?

jokram · 2017-01-24T12:54:39Z

@larsxschneider: No problem. I'm still analyzing.
Note: After merging the master branch further fixes are necessary before mering the pull request.
I'll let you know when it make sense to review the changes (I need to push new commits with some fixes).

larsxschneider · 2017-01-24T13:26:46Z

OK. Sounds good! Don't hesitate to reach out to me if you run into any trouble with the enterprise-config-for-git setup.

jokram · 2017-01-24T17:34:45Z

Concerning the proxy you might be right. But this will take a bit longer to understand, but it might be worth to avoid problems in the field.

I've created a DESIGN.md file to write down my analysis results. In addition this contains some design ideas you might have a look at (if the stress is decreasing a bit). But these ideas should be discussed first before continuing. Maybe it makes sense to cover this in a separate pull request or issue.

Require latest Git and Git LFS versions

jokram added 5 commits January 16, 2017 17:37

Add GITHUB_PROTOCOL support (http or https)

04f30e5

Replaced 'adsk' with where possible

4bd82c6

Replaced string 'Autodesk' with alternatives

82cebaf

Merge branch 'feature/configurable-protocol' into feature/configurabl…

fa9cb4b

…e-company

Renamed ADS_USER to GHE_USER and ADS_PASSWORD_OR_TOKEN to GHE_PASSWOR…

f159044

…D_OR_TOKEN

larsxschneider reviewed Jan 17, 2017

View reviewed changes

larsxschneider reviewed Jan 18, 2017

View reviewed changes

jokram added 5 commits January 18, 2017 16:53

Replaced GITHUB_* with GHE_* and renamed GITHUB_PROTOCOL to GHE_HTTP

f8a2ffc

Added KIT_COMPANY in enterprise.constants

3cf6856

Removed question from comment in setup.sh

29ccd48

Replaced GITHUB_PROTOCOL with GHE_HTTP in help.sh

2623cff

Merge branch 'master' into feature/configurable-company

2c45f28

jokram mentioned this pull request Jan 20, 2017

Add GITHUB_PROTOCOL support (http or https) #7

Closed

Fixed company in mkpr.sh, mkrepo.sh, paste.sh

ad878c8

jokram added 3 commits January 24, 2017 17:48

Added support for http_proxy in install-helper.ps1

722356e

Renamed TODO.md to DESIGN.md and added design ideas

5935dce

Added configuration data and some open points to DESIGN.md

c24bbe1

ballab1 pushed a commit to ballab1/enterprise-config-for-git that referenced this pull request Nov 24, 2019

Merge pull request Autodesk#8 from bellro/dev/bellro/c4dev_update

da986cc

Require latest Git and Git LFS versions

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make company configurable as far as possible #8

Make company configurable as far as possible #8

jokram commented Jan 17, 2017

larsxschneider Jan 17, 2017

jokram Jan 18, 2017

larsxschneider Jan 17, 2017

jokram Jan 18, 2017

larsxschneider Jan 18, 2017

jokram Jan 18, 2017

larsxschneider Jan 18, 2017

jokram Jan 18, 2017

larsxschneider Jan 18, 2017

larsxschneider Jan 18, 2017

jokram Jan 18, 2017

larsxschneider Jan 18, 2017

larsxschneider Jan 18, 2017

jokram Jan 18, 2017

larsxschneider Jan 18, 2017

larsxschneider commented Jan 24, 2017

jokram commented Jan 24, 2017

larsxschneider commented Jan 24, 2017

jokram commented Jan 24, 2017

Make company configurable as far as possible #8

Are you sure you want to change the base?

Make company configurable as far as possible #8

Conversation

jokram commented Jan 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

larsxschneider commented Jan 24, 2017

jokram commented Jan 24, 2017

larsxschneider commented Jan 24, 2017

jokram commented Jan 24, 2017