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

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 join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Development

No branches or pull requests

3 participants