Skip to content

Remove unnecesary contrib packages from docs install #1470

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

Merged

Conversation

NathanielRN
Copy link
Contributor

Description

As a follow up to #1464 I found that the docs tests pass just fine if there these packages are not installed.

Making this PR to mostly ask questions if anyone knows why these were needed!

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

tox -e docs passes and the sphinx build is created without a problem.

Does This PR Require a Contrib Repo Change?

  • No.

Checklist:

  • Followed the style guidelines of this project
    - [ ] Changelogs have been updated
    - [ ] Unit tests have been added
    - [ ] Documentation has been updated

@NathanielRN NathanielRN requested review from a team, owais and hectorhdzg and removed request for a team December 12, 2020 03:21
@ocelotl ocelotl changed the title Remove unnecesary ontrib packages from docs install Remove unnecesary contrib packages from docs install Dec 12, 2020
@lzchen
Copy link
Contributor

lzchen commented Dec 14, 2020

Wasn't this the fix for the original docs build failing in the first place?

@NathanielRN
Copy link
Contributor Author

@lzchen Originally the docs were failing because it couldn’t find a clone of the Contrib repo in its root directory (we made that during the GitHub action workflow but not this one). So the earlier fix downloaded them from Git instead of installing a local version of the package. But turns out all references to Contrib packages are in code examples not documented code.

@NathanielRN NathanielRN force-pushed the remove-unecessary-installs branch from 3abecd4 to 4796f7c Compare December 14, 2020 17:48
@lzchen
Copy link
Contributor

lzchen commented Dec 14, 2020

@NathanielRN
Did we remove all the code examples that had contrib related stuff in them? Can you link the PR?

@lzchen lzchen added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Dec 14, 2020
@NathanielRN NathanielRN force-pushed the remove-unecessary-installs branch from 862745f to c552011 Compare December 14, 2020 21:31
@NathanielRN NathanielRN force-pushed the remove-unecessary-installs branch from c552011 to 7ff0509 Compare December 14, 2020 21:32
@NathanielRN
Copy link
Contributor Author

@lzchen

We didn't have to remove them, the Contrib examples are still here in Core as referenced in Getting Started and as found in the docs/examples/ folder.

I think we just assumed that we needed to download the instrumentations for these examples, but because these examples aren't getting auto-documented themselves (they're just being pointed to as files), they don't need to be referenced explicitly.

@NathanielRN NathanielRN force-pushed the remove-unecessary-installs branch from 98ee252 to 7ff0509 Compare December 14, 2020 21:39
Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

LGTM

@codeboten codeboten merged commit 33fa7b9 into open-telemetry:master Dec 14, 2020
@NathanielRN NathanielRN deleted the remove-unecessary-installs branch December 14, 2020 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants