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

Move all DD code into its own directory #6

Merged
merged 1 commit into from
Apr 8, 2020
Merged

Move all DD code into its own directory #6

merged 1 commit into from
Apr 8, 2020

Conversation

ocelotl
Copy link
Contributor

@ocelotl ocelotl commented Mar 26, 2020

Move all DD code into its own directory. This is done to keep the code close at hand for instrumentor development.

Fixes #3

Copy link
Member

@andrewhsu andrewhsu left a comment

Choose a reason for hiding this comment

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

LGTM

this is helpful to get stuff started to add instrumentations like #8

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.

Thanks for taking on this work. One change I'd like to request is that this change is doing more than moving dd code, those changes would be better done separately, otherwise it makes it hard to know what's changed.

One question: is there a reason to add the opentelemetry-auto-instrumentation code in this repo? I thought that would continue to live in the main opentelemetry-python repo.

@ocelotl ocelotl changed the title Move all DD code into dd Remove all DD code Apr 2, 2020
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.

Can you add a comment in the README regarding the initial code being available as reference in the reference branch? This would help folks taking a look at the repo for the first time. A few files were deleted that shouldn't be. Please fix.

CODEOWNERS Outdated Show resolved Hide resolved
LICENSE Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@ocelotl
Copy link
Contributor Author

ocelotl commented Apr 4, 2020

Can you add a comment in the README regarding the initial code being available as reference in the reference branch? This would help folks taking a look at the repo for the first time. A few files were deleted that shouldn't be. Please fix.

Thanks for the review, I have added the changes 👍

@andrewhsu
Copy link
Member

Had a chat with @ocelotl @codeboten and @mauriciovasquezbernal and we're going to pause on this PR to make the porting of the instrumentation PRs easier.

@ocelotl ocelotl changed the title Remove all DD code Move all DD code into its own directory Apr 8, 2020
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 5aee3ce into open-telemetry:master Apr 8, 2020
NathanielRN pushed a commit to NathanielRN/opentelemetry-python-contrib-1 that referenced this pull request Oct 17, 2020
* Add .pylintrc based on OpenCensus (related open-telemetry#6).

* Fix/disable pylint warnings.

* pylint: Fix W0107: Unnecessary pass statement (unnecessary-pass)

I'm not sure I like this warning.
NathanielRN pushed a commit to NathanielRN/opentelemetry-python-contrib-1 that referenced this pull request Oct 20, 2020
* Add .pylintrc based on OpenCensus (related open-telemetry#6).

* Fix/disable pylint warnings.

* pylint: Fix W0107: Unnecessary pass statement (unnecessary-pass)

I'm not sure I like this warning.
NathanielRN pushed a commit to NathanielRN/opentelemetry-python-contrib-1 that referenced this pull request Oct 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove DD code
3 participants