Skip to content

docs/contribution-guidelines: improve formatting, remove specifics of tests#3879

Merged
estroz merged 1 commit intooperator-framework:masterfrom
estroz:docs/remove-test-description
Sep 24, 2020
Merged

docs/contribution-guidelines: improve formatting, remove specifics of tests#3879
estroz merged 1 commit intooperator-framework:masterfrom
estroz:docs/remove-test-description

Conversation

@estroz
Copy link
Copy Markdown
Member

@estroz estroz commented Sep 15, 2020

Description of the change:

  • docs/contribution-guidelines: improve formatting, remove specifics of tests and move simplified test doc up a level, change file names to be indicative of contents

Motivation for the change: these changes were made with the goal of simplifying contribution information, mainly removing test descriptions. These descriptions tend to drift because tests change often, so docs shouldn't discuss specifics of tests, only how to run them. This PR also fixes a broken link implicitly.

/kind documentation

Checklist

If the pull request includes user-facing changes, extra documentation is required:

@openshift-ci-robot openshift-ci-robot added the kind/documentation Categorizes issue or PR as related to documentation. label Sep 15, 2020
@estroz
Copy link
Copy Markdown
Member Author

estroz commented Sep 15, 2020

/cc @theishshah @joelanford

Comment thread website/content/en/docs/contribution-guidelines/testing.md
@estroz estroz closed this Sep 15, 2020
@estroz estroz reopened this Sep 15, 2020
@estroz estroz closed this Sep 15, 2020
@estroz estroz reopened this Sep 15, 2020
@estroz estroz force-pushed the docs/remove-test-description branch 3 times, most recently from 6408aa2 to c760f8a Compare September 15, 2020 22:21
Comment thread website/content/en/docs/contribution-guidelines/developer-guide.md Outdated
- `DOCKER_CLI_EXPERIMENTAL` set to `enabled`
- `COVERALLS_TOKEN` token to integrate the project with `https://coveralls.io/`. So, enable your fork in `https://coveralls.io/` and generate a token to allow it.
The operator-sdk repo uses [Travis CI][travis] to test each pull request and build images for both master commits
and releases. You can alter these processes by modifying this [`.travis.yml`][travis.yml] config file.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

isn't this kind of weird advice? If i'm going to be maintaining a fork, i don't want to have a floating commit that I need to keep on top with my .travis.yaml changes, no?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Why would you need a special commit for your fork? You are making these changes with the intent of them being in SDK master, so once that is done you ff your fork.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh, I misunderstood what you meant by "You can alter these processes..."

Comment on lines +34 to -76
## Continuous Integration (CI)

- `ANSIBLE_IMAGE` docker image name (e.g. `quay.io/joelanford/ansible-operator`)
- `HELM_IMAGE` same as above, but for helm
- `SCORECARD_PROXY_IMAGE` same as above, but for scorecard proxy
- `DOCKER_USERNAME` credentials for your repo
- `DOCKER_PASSWORD` credentials for your repo
- `DOCKER_CLI_EXPERIMENTAL` set to `enabled`
- `COVERALLS_TOKEN` token to integrate the project with `https://coveralls.io/`. So, enable your fork in `https://coveralls.io/` and generate a token to allow it.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How a collab will know how to set up the Travis in their fork if we are removing the info?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

These aren't being removed, see below.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry, I did not find where we are describing the ENV Vars that should be set in the fork travis. Could you please link it here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Comment thread website/content/en/docs/contribution-guidelines/local-changes.md
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 17, 2020
@estroz estroz force-pushed the docs/remove-test-description branch from c760f8a to 04d700b Compare September 17, 2020 16:36
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 17, 2020
@jberkhahn
Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 18, 2020
@estroz estroz merged commit f7ad625 into operator-framework:master Sep 24, 2020
@estroz estroz deleted the docs/remove-test-description branch September 24, 2020 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/documentation Categorizes issue or PR as related to documentation. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants