Skip to content
This repository was archived by the owner on Mar 12, 2025. It is now read-only.

Timer logging should be switched to a flag. #1081

Open
Tracked by #1074 ...
sunrabbit123 opened this issue Sep 8, 2023 · 10 comments
Open
Tracked by #1074 ...

Timer logging should be switched to a flag. #1081

sunrabbit123 opened this issue Sep 8, 2023 · 10 comments
Labels
good first issue Good for newcomers

Comments

@sunrabbit123
Copy link
Collaborator

sunrabbit123 commented Sep 8, 2023

  1. the function is platform dependent
  2. if you want to bind to multiple platforms, this function can be a headache.
  3. when not in debug mode, it times but does not log.

https://github.com/dudykr/stc/blob/main/crates/stc_ts_type_checker/src/lib.rs#L91-L107


Suggest solution

mapping function, and take flag

@sunrabbit123
Copy link
Collaborator Author

part of resolve #1085

not implemented flag system

@moaoa
Copy link

moaoa commented Oct 14, 2023

hi there I am new to rust

  pub fn check(&self, entry: Arc<FileName>) -> ModuleId {
        let timer = PerfTimer::tracing_debug();

        let modules = self.module_loader.load_module(&entry, true).expect("failed to load entry");
        #[cfg(debug_assertions)]
        timer.log(&format!("Loading of `{}` and dependencies", entry));
        let timer = PerfTimer::tracing_debug();

        self.analyze_module(None, entry.clone());
        #[cfg(debug_assertions)]
        timer.log(&format!("Analysis of `{}` and dependencies", entry));

        modules.entry.id
    }

should the changes be like this?
@sunrabbit123

@moaoa
Copy link

moaoa commented Oct 14, 2023

or should i use #[cfg(feature = "debug")] ?

@sunrabbit123
Copy link
Collaborator Author

It's a similar feeling, but I hope that the ratings for debug, information, error, etc. will be divided

@moaoa
Copy link

moaoa commented Oct 14, 2023

Do you mean we need to create another flag for logging?

@moaoa
Copy link

moaoa commented Oct 14, 2023

Do you mean we need to create another flag for logging?

Something like this:

#[cfg(feature = "log")]

@sunrabbit123
Copy link
Collaborator Author

I know that you can't assign a value to a feature flag.
Is there a workaround?

What I'm envisioning is to divide the log levels and ignore logs below a certain level by a certain value at compile time.

Of course, the above is just my wishful thinking.

The purpose of the feature is to make it work without bugs in NAPI, WASM, and not to have logs that are not needed.

And just in case it's confusing, we'll mention the
When I refer to flags in the text, I mean argument values for something, not rust feature flags.
Sure, it could be a rust feature flag, but I think it has limited functionality.

@kdy1
Copy link
Member

kdy1 commented Oct 16, 2023

Yeah, feature flags are quite limited. We may create our own macro for logging which invokes macros from tracing.
If we do so, we will have much more detailed control, even with cargo feature flags.

cargo feature:

  • stc_logger/log-type-creation
  • stc_logger/info (maybe)

@moaoa
Copy link

moaoa commented Oct 23, 2023

Can you guys guide me on how to implement this?
@kdy1
@sunrabbit123

@sunrabbit123
Copy link
Collaborator Author

What guide do you want?
I didn't understand what you were talking about.


https://docs.rs/log/latest/log/

btw, The following structure also looks good.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue Good for newcomers
Development

No branches or pull requests

3 participants