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

Add vanity imports to internal packages #2280

Merged

Conversation

mx-psi
Copy link
Member

@mx-psi mx-psi commented Oct 7, 2021

This adds vanity imports on internal files, excluding autogenerated and third party files. It was generated by running porto -w --include-internal . with porto version 0.4.0.

It fixes #2279.

@mx-psi
Copy link
Member Author

mx-psi commented Oct 7, 2021

Looks like porto also needs to skip autogenerated files.

@MrAlias
Copy link
Contributor

MrAlias commented Oct 21, 2021

Is this still being worked on? It seems like the upstream PR mentioned was merged.

@MrAlias MrAlias added the response needed Waiting on user input before progress can be made label Oct 21, 2021
@mx-psi
Copy link
Member Author

mx-psi commented Oct 21, 2021

@MrAlias I still need to add support to skip third_party folders to porto for this to make sense, will get around to do it in the near future :)

@MrAlias MrAlias removed the response needed Waiting on user input before progress can be made label Oct 21, 2021
@MovieStoreGuy
Copy link

@mx-psi does this need to be done all at once, or can it be done in stages?

@mx-psi

This comment has been minimized.

@mx-psi
Copy link
Member Author

mx-psi commented Oct 27, 2021

does this need to be done all at once, or can it be done in stages?

@MovieStoreGuy We can do it on stages if we exclude directories, happy to do it in whichever way works best for you

@jcchavezs
Copy link
Contributor

jcchavezs commented Oct 27, 2021

does this need to be done all at once, or can it be done in stages?

I think the easiest is to do all at once so next PRs can have the check, doing it progressively might take longer and there will be some rework as new added code in the meantime can go without vanity imports.

@jcchavezs
Copy link
Contributor

@mx-psi 0.4.0 is out https://github.com/jcchavezs/porto/tree/v0.4.0. Thank you!

@codecov
Copy link

codecov bot commented Oct 27, 2021

Codecov Report

Merging #2280 (f3632cd) into main (ef0cdeb) will decrease coverage by 0.0%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #2280     +/-   ##
=======================================
- Coverage   73.6%   73.6%   -0.1%     
=======================================
  Files        175     175             
  Lines      12419   12419             
=======================================
- Hits        9147    9144      -3     
- Misses      3039    3041      +2     
- Partials     233     234      +1     
Impacted Files Coverage Δ
bridge/opencensus/internal/oc2otel/attributes.go 100.0% <ø> (ø)
bridge/opencensus/internal/oc2otel/span_context.go 100.0% <ø> (ø)
...pencensus/internal/oc2otel/tracer_start_options.go 100.0% <ø> (ø)
bridge/opencensus/internal/otel2oc/span_context.go 100.0% <ø> (ø)
bridge/opencensus/internal/span.go 100.0% <ø> (ø)
bridge/opencensus/internal/tracer.go 100.0% <ø> (ø)
bridge/opentracing/internal/mock.go 67.0% <ø> (ø)
.../otlp/otlpmetric/internal/connection/connection.go 3.4% <ø> (ø)
...p/otlpmetric/internal/metrictransform/attribute.go 100.0% <ø> (ø)
...otlp/otlpmetric/internal/metrictransform/metric.go 79.0% <ø> (ø)
... and 38 more

@MrAlias MrAlias added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Oct 29, 2021
@MrAlias MrAlias merged commit 19294aa into open-telemetry:main Oct 29, 2021
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.

Vanity imports are not checked on internal packages
5 participants