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

Setup docs: Add installation instructions for Debian and Fedora OS #8491 #8486

Merged
merged 1 commit into from
Feb 21, 2018
Merged

Setup docs: Add installation instructions for Debian and Fedora OS #8491 #8486

merged 1 commit into from
Feb 21, 2018

Conversation

0xTheProDev
Copy link
Contributor

@0xTheProDev 0xTheProDev commented Feb 19, 2018

Sepearte package available for Google cloud Java API plugin. For Linux system, the package can be directly installed.

Signed-off-by: Progyan Bhattacharya progyanb@acm.org

Fixes #8491

PR Checklist

Ensure that you have:

  • Read and understood our PR guideline: https://github.com/TEAMMATES/teammates/blob/master/docs/process.md#step-4-submit-a-pr
    • Added the issue number to the "Fixes" keyword above
    • Titled the PR as specified in the abovementioned document
  • Made your changes on a branch other than master and release
  • Gone through all the changes in this PR and ensured that:
    • They addressed one (and only one) issue
    • No unintended changes were made
  • Run and passed static analysis: ./gradlew lint and npm run lint
  • Added/updated tests, if changes in functionality were involved
  • Added/updated documentation to public APIs (classes, methods, variables), if applicable

Outline of Solution

@teammates-bot
Copy link

Hi @Progyan1997, these parts of your pull request do not appear to follow our contributing guidelines:

  1. PR Title
    • Issue Reference (#<issue-number>) missing.
  2. PR Description
    • Issue Reference (#<issue-number>) missing.

1 similar comment
@teammates-bot
Copy link

Hi @Progyan1997, these parts of your pull request do not appear to follow our contributing guidelines:

  1. PR Title
    • Issue Reference (#<issue-number>) missing.
  2. PR Description
    • Issue Reference (#<issue-number>) missing.

@tshradheya
Copy link
Member

@Progyan1997 TEAMMATES follows the following process for contribution.

So before you open a PR, it would be necessary to open an issue about what you aim to do via the PR and get the approval from the project maintainers that the issue is valid and needs to be worked upon.

@0xTheProDev
Copy link
Contributor Author

@tshradheya This PR improves the Documentation just a bit. This does not help anything with Design or Development of the project.
PS: I can open an issue if it's necessary.

@tshradheya
Copy link
Member

Yes, please open an issue. It doesn't matter that even if it is just for Documentation

@teammates-bot
Copy link

Hi @Progyan1997, these parts of your pull request do not appear to follow our contributing guidelines:

  1. PR Title
    • Issue Reference (#<issue-number>) missing.
  2. PR Description

@teammates-bot
Copy link

Hi @Progyan1997, these parts of your pull request do not appear to follow our contributing guidelines:

  1. PR Title
  2. PR Description

@0xTheProDev 0xTheProDev changed the title Docs: Environment Setup: Update Package Detail [#8491] Docs: Environment Setup: Update Package Detail Feb 20, 2018
@teammates-bot
Copy link

Hi @Progyan1997, these parts of your pull request do not appear to follow our contributing guidelines:

  1. PR Description

@0xTheProDev 0xTheProDev changed the title [#8491] Docs: Environment Setup: Update Package Detail Docs: Environment Setup: Update Package Detail #8491 Feb 20, 2018
@teammates-bot
Copy link

Hi @Progyan1997, these parts of your pull request do not appear to follow our contributing guidelines:

  1. PR Description

@wkurniawan07
Copy link
Member

Hi @Progyan1997, thanks for the contribution.
However, this does not look like something we really need because the current steps as detailed in the setup document works also for Linux machines, and we don't want to be duplicates of the official setup guide from the Cloud SDK themselves (otherwise we also have to document the apt-get alternative and all the what-nots, right?).

@0xTheProDev
Copy link
Contributor Author

@wkurniawan07 I am already working on apt for Debian/Ubuntu systems. And the given setup no longer works with RHEL-based systems (at least)

@wkurniawan07
Copy link
Member

@Progyan1997 Are you sure just downloading the Java SDK works?

@0xTheProDev
Copy link
Contributor Author

@wkurniawan07 Yes, that's the way I followed to work with!

@wkurniawan07
Copy link
Member

@Progyan1997 as in you don't need to install the Cloud SDK first?

@0xTheProDev
Copy link
Contributor Author

@wkurniawan07 You must install gcloud package first, although if you don't it will install it anyway for the dependencies. But my point was gcloud is not allowed to install java-plugin, it must be installed separately as a package.

@wkurniawan07
Copy link
Member

@Progyan1997 are you sure the install commands with apt-get or yum work just like that without setting up the configurations?

@0xTheProDev
Copy link
Contributor Author

@wkurniawan07 Please explain what you said.

@wkurniawan07
Copy link
Member

According to this and this (the "Before you begin" part) there are some things that need to be done.

@0xTheProDev
Copy link
Contributor Author

@wkurniawan07 That is for installing gcloud, the parent module. I think it's already present in the documentation.

@wkurniawan07
Copy link
Member

@Progyan1997 it is not; our current setup docs make no mention to yum/apt-get-based installation other than a reference to the official installation page. So, installing app-engine-java with apt-get or yum works without the need for the configuration set out in the "Before you begin" part?

@0xTheProDev
Copy link
Contributor Author

@wkurniawan07 Then should I add those information as well?

@wkurniawan07
Copy link
Member

Before you proceed further, have you tried that the entire setup (up to building/testing/deploying) works in any one of those OSes?
The setting up/development guideline is field-tested and actively maintained for Linux (Travis CI, although strictly speaking it's Ubuntu Trusty), Windows (AppVeyor), and Mac OS (my own setup). If we want to add support for any new OS, we need to make sure that (1) the entire routine works and (2) the setup is at least used by someone. We don't want to end up adding something now only for someone later to complain "this instruction does not work for me" and we don't have any resource/know-how to help.

@whipermr5 whipermr5 added the s.Ongoing The PR is being worked on by the author(s) label Feb 20, 2018
@0xTheProDev
Copy link
Contributor Author

@wkurniawan07

Before you proceed further, have you tried that the entire setup (up to building/testing/deploying) works in any one of those OSes?

Yes, I did indeed. The given procedure did not work with CentOS 7, at least that was my experience. So I had to follow the official documentation and proceed with the mentioned steps. Since I am adding yum support, I thought of providing apt package informations as well.

We don't want to end up adding something now only for someone later to complain "this instruction does not work for me"

The previous documentations are kept as is, because most probably it works with other Linux distributions as well as Windows. Personally I would recommend to follow them, and then mine only if it does not work.

@wkurniawan07
Copy link
Member

I'd recommend the following:

  • In the step which installs Google Cloud SDK, add a reference to the official docs for Debian/Ubuntu/Red Hat/CentOS. Something like "If you use Debian/Ubuntu/Red Hat/CentOS, the installation is slightly more complicated. Refer to the official documentation provided by Google.". It is too wordy for everything to be duplicated here.
  • In the step which installs App Engine Java, it is acceptable to mention yum/apt-get provided the above is done, but you don't need new code blocks for that (and the header is rather disruptive). Just do something like:
    # Linux/OS X/Windows
    gcloud -q components install app-engine-java
    
    # Debian/Ubuntu
    sudo apt-get install google-cloud-sdk-app-engine-java

@0xTheProDev
Copy link
Contributor Author

@wkurniawan07

In the step which installs Google Cloud SDK, add a reference to the official docs...
... you don't need new code blocks for that...

Made the above changes as you recommended.

Copy link
Member

@wkurniawan07 wkurniawan07 left a comment

Choose a reason for hiding this comment

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

  • You're still mixing up installing Cloud SDK and installing App Engine Java.
  • Let's not use abbreviation for "Red Hat".

@wkurniawan07 wkurniawan07 self-assigned this Feb 21, 2018
@0xTheProDev
Copy link
Contributor Author

@wkurniawan07 Have a look.

sudo yum install google-cloud-sdk

# Debian/Ubuntu
sudo apt-get install google-cloud-sdk
Copy link
Member

Choose a reason for hiding this comment

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

No need to include them for the following reasons:

  • They are mentioned in the official docs
  • If you only display them like this, it's unlikely to work because the "Before you begin" steps are not done

If anything, just do what I suggested: "If you use Debian/Ubuntu/Red Hat/CentOS, the installation is slightly more complicated. Refer to the official documentation provided by Google."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay 👍

```
If you face any problem regarding the installation, refer to the official documentation of Google Cloud Engine for [Debian/Ubuntu](https://cloud.google.com/sdk/docs/quickstart-debian-ubuntu) or [RHEL/CentOS/Fedora](https://cloud.google.com/sdk/docs/quickstart-redhat-centos).
Copy link
Member

Choose a reason for hiding this comment

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

This is about installing the Cloud SDK; if everything was done right before, the yum and apt-get commands down here would have worked.

# Windows
install.bat --path-update true
```
If you are installing in Red Hat/CentOS/Fedora/Debian/Ubuntu follow the official documentation of Google Cloud Engine for [Debian/Ubuntu](https://cloud.google.com/sdk/docs/quickstart-debian-ubuntu) or [RHEL/CentOS/Fedora](https://cloud.google.com/sdk/docs/quickstart-redhat-centos) respectively.
Copy link
Member

Choose a reason for hiding this comment

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

Minor readability tweaks:

  • Red Hat/CentOS/Fedora/Debian/Ubuntu -> Red Hat, CentOS, Fedora, Debian, or Ubuntu
  • Ubuntu follow -> Ubuntu, refer to
  • official documentation -> quickstart guide
  • Google Cloud Engine -> Google Cloud SDK
  • RHEL -> Red Hat

Copy link
Contributor Author

@0xTheProDev 0xTheProDev Feb 21, 2018

Choose a reason for hiding this comment

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

@wkurniawan07 Done 😄

@wkurniawan07 wkurniawan07 changed the title Docs: Environment Setup: Update Package Detail #8491 Setup docs: Add installation instructions for Debian and Fedora OS #8491 Feb 21, 2018
@teammates-bot
Copy link

Hi @Progyan1997, these parts of your pull request do not appear to follow our contributing guidelines:

  1. PR Description

Copy link
Member

@wkurniawan07 wkurniawan07 left a comment

Choose a reason for hiding this comment

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

LGTM

Sepearte package available for Google cloud Java API plugin. For Linux system, the package can be directly installed for both yum and apt.

Signed-off-by: Progyan Bhattacharya <progyanb@acm.org>
@wkurniawan07 wkurniawan07 merged commit 3a330e6 into TEAMMATES:master Feb 21, 2018
@wkurniawan07 wkurniawan07 added s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging and removed s.Ongoing The PR is being worked on by the author(s) labels Feb 21, 2018
@whipermr5 whipermr5 added this to the V6.3.0 milestone Feb 22, 2018
@whipermr5 whipermr5 added e.1 c.DevOps Process-related or build-related improvement and addition labels Feb 22, 2018
@0xTheProDev 0xTheProDev deleted the progyan-001 branch March 17, 2018 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c.DevOps Process-related or build-related improvement and addition s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setup docs: Add installation instructions for Debian and Fedora OS
5 participants