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

Fix/the entry using log #100

Merged
merged 14 commits into from
Apr 26, 2024
Merged

Fix/the entry using log #100

merged 14 commits into from
Apr 26, 2024

Conversation

Joinhack
Copy link
Collaborator

@Joinhack Joinhack commented Apr 4, 2024

improve:

  1. Currently, we are using eprintln! or println! to output informational and error messages before initializing the logger. However, as the logger is configured using a configuration file, there's a need to print information before that, such as errors encountered during tasks like checking checksum files. To enhance this process, we propose defining a new logging macro that allows us to utilize logging before the logger initialization, thereby replacing the direct use of eprintln! and println! macros.
  2. add comments.
  3. improve the dependency file.
  4. run the multi-thread api crash for newest api.
  5. refactor the blockless context remove older api.

@Joinhack Joinhack requested a review from dmikey April 4, 2024 01:02
@coveralls
Copy link

coveralls commented Apr 4, 2024

Pull Request Test Coverage Report for Build 8795650795

Details

  • 114 of 280 (40.71%) changed or added relevant lines in 22 files are covered.
  • 21 unchanged lines in 11 files lost coverage.
  • Overall coverage increased (+0.09%) to 34.884%

Changes Missing Coverage Covered Lines Changed/Added Lines %
blockless/src/lib.rs 13 14 92.86%
crates/blockless-drivers/src/cdylib_driver/driver_wasi_file.rs 0 1 0.0%
crates/blockless-drivers/src/wasi/memory.rs 0 1 0.0%
bls-runtime/src/v86.rs 0 2 0.0%
crates/blockless-drivers/src/cgi_driver/mod.rs 0 2 0.0%
crates/blockless-drivers/src/wasi/mod.rs 0 2 0.0%
bls-runtime/src/v86config.rs 2 5 40.0%
crates/blockless-drivers/src/cgi_driver/db.rs 14 17 82.35%
bls-runtime/src/config.rs 18 22 81.82%
crates/wasi-common/src/ctx.rs 1 5 20.0%
Files with Coverage Reduction New Missed Lines %
crates/blockless-drivers/src/cgi_driver/db.rs 1 89.15%
crates/blockless-drivers/src/error.rs 1 0.0%
crates/blockless-drivers/src/http_driver/mod.rs 1 0.0%
crates/blockless-drivers/src/cgi_driver/mod.rs 1 0.0%
bls-runtime/src/main.rs 1 38.86%
bls-runtime/src/config.rs 2 78.24%
blockless/src/context.rs 2 30.0%
crates/blockless-drivers/src/http_driver/reqwest_driver.rs 2 47.76%
../../../.cargo/registry/src/index.crates.io-6f17d22bba15001f/log-0.4.21/src/lib.rs 3 4.76%
crates/wasi-common/src/ctx.rs 3 53.16%
Totals Coverage Status
Change from base Build 8795586814: 0.09%
Covered Lines: 4763
Relevant Lines: 13654

💛 - Coveralls

}};

() => {{
use log::Level;
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe import this at top level? otherwise this may be imported every time we log.
Im not sure if compiler optimizes this away though.

}};

() => {{
use log::Level;
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here.

use env_logger::Logger;
use log::{Level, Log, MetadataBuilder, Record};

static mut ENV_LOGGER: Option<Logger> = None;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Lets try not use an option type - but instead use the concrete type directly.
We can use OnceLock for this.

Example (i'd envision something like this):

static mut ENV_LOGGER: std::sync::OnceLock<Logger> = std::sync::OnceLock::new();
pub fn env_logger() -> &'static mut Logger {
    ENV_LOGGER.get_or_init(|| { ... })
}

@zees-dev
Copy link
Collaborator

zees-dev commented Apr 4, 2024

I see few PRs where we make formatting changes - maybe we should enforce formatting in CI to ensure formatting before we push (cargo fmt --all --check).

Copy link
Collaborator

@zees-dev zees-dev left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@Joinhack Joinhack closed this Apr 23, 2024
@Joinhack Joinhack deleted the fix/the_entry_using_log branch April 23, 2024 06:08
@Joinhack Joinhack restored the fix/the_entry_using_log branch April 23, 2024 06:08
@Joinhack Joinhack reopened this Apr 23, 2024
@Joinhack Joinhack merged commit eb2b779 into main Apr 26, 2024
10 checks passed
@Joinhack Joinhack deleted the fix/the_entry_using_log branch July 16, 2024 23:09
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.

3 participants