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.

Sign up for GitHub

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

Jump to bottom

Remove log renamings of tracing crate #74747

Closed
3 of 36 tasks
oli-obk opened this issue Jul 25, 2020 · 15 comments · Fixed by #75514
Closed
3 of 36 tasks

Remove log renamings of tracing crate #74747

oli-obk opened this issue Jul 25, 2020 · 15 comments · Fixed by #75514
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Jul 25, 2020 •
edited
Loading

In #74726 I made a dependency import hack to reduce the diff size. We should replace all of these imports with tracing = "0.1" and then adjust the crate source by replacing all uses of log imports with tracing import.

Ideally only a few of these are done at a time to keep conflicts at a minimum and get such PRs merged fast.

Note: do not start working on this until #74726 is merged.
@oli-obk oli-obk added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Jul 25, 2020
@jonas-schievink jonas-schievink added the C-cleanup Category: PRs that clean code up or issues documenting cleanup. label Jul 25, 2020
@JohnTitor JohnTitor added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 25, 2020
@oli-obk oli-obk changed the title Remove from log renaming of tracing crate Remove log renamings of tracing crate Jul 25, 2020
@bishtpawan
Copy link
Contributor

Hi @oli-obk I saw your PR of (#74726 ) where you have replaced log="0.4" to log = { package = "tracing", version = "0.1" } and in this issue, you have mentioned like we have to replace all log imports to tracing imports. My question here is when you have already changed the package from log to tracing so now what left in it because the import name cannot be changed in files if we have used log as a crate name in Cargo.toml?
Do we need to replace the name of dependency from log to tracing then we'll clean up the code by replacing the log imports with tracing imports in the corresponding files? Or is there anything else you want?

Thanks,
Waiting for your response.

@oli-obk
Copy link
Contributor Author

oli-obk commented Jul 26, 2020

Do we need to replace the name of dependency from log to tracing then we'll clean up the code by replacing the log imports with tracing imports in the corresponding files? Or is there anything else you want?

Yes, that's exactly it. Replace log = { package = "tracing", version = "0.1" } with tracing = "0.1" and then fix up the errors by changing the import names. Note that the PR isn't merged yet, so this issue is not yet actionable.

@bishtpawan
Copy link
Contributor

Thanks for the response. Yes, I'm waiting for your PR to be merged first.

@georgio
Copy link
Contributor

georgio commented Jul 31, 2020

I would like to give this a shot when #74726 is merged

@oli-obk
Copy link
Contributor Author

oli-obk commented Jul 31, 2020

Once the PR is merged, before you work on anything, please post here which libs you are going to work on so that you don't conflict with others who are doing the same

@pawanbisht62
Copy link
Contributor

pawanbisht62 commented Aug 1, 2020 •
edited
Loading

As they are independent, we can make changes on multiple files right? If yes then I'll work on following libs

  • librustc_ast_lowering
  • librustc_ast_passes
  • librustc_ast_pretty
  • librustc_builtin_macros
  • librustc_codegen_llvm
  • librustc_codegen_ssa
  • librustc_data_structures)
    After that will pick new libs accordingly if we will left with some.
    Thanks

@gdhuper
Copy link
Contributor

gdhuper commented Aug 2, 2020 •
edited
Loading

If no one else is planning on tackling the following:

  • librustc_session
  • librustc_span
  • librustc_symbol_mangling
  • librustc_target
  • librustc_trait_selection
  • librustc_traits
  • librustc_ty
  • librustc_typeck
  • compiletest
    I would like to take a stab. Thanks

@georgio
Copy link
Contributor

georgio commented Aug 2, 2020 •
edited
Loading

I could take care of the remaining files:

  • librustc_ast
  • librustc_data_structures
  • librustc_driver
  • librustc_errors
  • librustc_expand
  • librustc_hir
  • librustc_incremental
  • librustc_infer
  • librustc_interface
  • librustc_lint
  • librustc_metadata
  • librustc_middle
  • librustc_mir
  • librustc_mir_build
  • librustc_parse
  • librustc_passes
  • librustc_privacy
  • librustc_query_system
  • librustc_resolve
  • librustc_save_analysis

@pawanbisht62 pawanbisht62 mentioned this issue Aug 2, 2020
Merged
JohnTitor added a commit to JohnTitor/rust that referenced this issue Aug 2, 2020
@JohnTitor
Rollup merge of rust-lang#75041 - pawanbisht62:feature/replace-log-wi…
e728848 
…th-tracing, r=oli-obk

Replaced log with tracing crate

Issue rust-lang#74747
@memoryruins memoryruins mentioned this issue Aug 3, 2020
Merged
@gdhuper gdhuper mentioned this issue Aug 3, 2020
Closed
32 tasks
@georgio
Copy link
Contributor

georgio commented Aug 4, 2020

Hey everyone, I would like to apologize for not being able to carry this through at the moment. My country was rocked by a massive explosion and I have to be on the ground to help. I wouldn't mind getting back to this as soon as things calm down a bit but if you're short on time I would suggest that someone else takes care of my part of the issue.

@gdhuper
Copy link
Contributor

gdhuper commented Aug 4, 2020

Hey everyone, I would like to apologize for not being able to carry this through at the moment. My country was rocked by a massive explosion and I have to be on the ground to help. I wouldn't mind getting back to this as soon as things calm down a bit but if you're short on time I would suggest that someone else takes care of my part of the issue.

Hey @georgio, so sorry to hear that. I can take care of the rest.

@georgio
Copy link
Contributor

georgio commented Aug 5, 2020

@gdhuper thank you very much. I really appreciate it.

@pawanbisht62 pawanbisht62 mentioned this issue Aug 5, 2020
Merged
@CloudNClock
Copy link

CloudNClock commented Aug 5, 2020 •
edited
Loading

I will take care of librustdoc, then every task is claimed if I am not mistaken.
Stay safe.

It seems librustdoc has been updated already.

@gdhuper gdhuper mentioned this issue Aug 14, 2020
Merged
32 tasks
@bors bors closed this as completed in 9b88e0a Aug 15, 2020
@pawanbisht62
Copy link
Contributor

@bors can you please look at the PR #75187 as it is related to this issue and still in open state. Thanks

@Aaron1011 Aaron1011 reopened this Aug 17, 2020
@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 17, 2020

@pawanbisht62 bors is a bot, You can just post messages like the one you did in this issue without pinging it. We just need to ping it when giving it commands to merge PRs

bors added a commit to rust-lang-ci/rust that referenced this issue Aug 17, 2020
@bors
Auto merge of rust-lang#75187 - pawanbisht62:feature/incorporate-trac…
8d185ca 
…ing, r=oli-obk

Incorporated Tracing Crate in some libraries

Issue rust-lang#74747
@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 26, 2020

I think we solved all of these

@oli-obk oli-obk closed this as completed Aug 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Replaced log with tracing gdhuper/rust-1
Replacing log with tracing gdhuper/rust-1
9 participants
@oli-obk @Aaron1011 @jonas-schievink @gdhuper @georgio @JohnTitor @CloudNClock @pawanbisht62 @bishtpawan