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

Refactor logger class impl with std::filesystem #618

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

sksat
Copy link
Collaborator

@sksat sksat commented Feb 19, 2024

Related issues

N/A

Description

Test results

logger directory & log files creation test

$ rm -rf data/sample/logs # to test logs directory creation
$ cmake -B build
$ cmake --build build
$ mkdir build/hoge
$ cd build/hoge
$ ../S2E
$ cd ../../
$ ls data/sample/logs
logs_240219_172539
$ ls data/sample/logs/logs_240219_172539/ # test logs dir files were created
240219_172539_default.csv      gnss_receiver.ini              magnetorquer.ini              sample_satellite.ini        sun_sensor.ini
angular_velocity_observer.ini  ground_station_antenna.ini     reaction_wheel.ini            sample_simulation_base.ini  thruster.ini
attitude_observer.ini          ground_station_calculator.ini  sample_disturbance.ini        sample_structure.ini        torque_generator.ini
component_interference.ini     gyro_sensor.ini                sample_ground_station.ini     spacecraft_antenna.ini
force_generator.ini            magnetometer.ini               sample_local_environment.ini  star_sensor.ini

Impact

  • Logger class implementation
  • Logger class interface
    • std::string & std::filesystem::path can be converted, so the actual change cost is very small

@sksat sksat added priority::medium priority medium library library minor update add functionality in a backwards compatible manner labels Feb 19, 2024
@sksat sksat self-assigned this Feb 19, 2024
@sksat sksat requested a review from a team as a code owner February 19, 2024 08:43
@sksat sksat requested review from seki-hiro, t-hosonuma and conjikidow and removed request for a team February 19, 2024 08:43
@sksat sksat force-pushed the feature/refactor-logger-by-std-filesystem branch from 9173893 to cd90248 Compare February 19, 2024 08:46
@sksat sksat force-pushed the feature/refactor-logger-by-std-filesystem branch from bc77139 to 34eac33 Compare February 19, 2024 08:56
@sksat sksat marked this pull request as draft February 19, 2024 08:56
@sksat sksat marked this pull request as ready for review February 19, 2024 08:59
@200km
Copy link
Member

200km commented Feb 20, 2024

修正提案ありがとうございます。ただ、Major Update v8.0.0に関するPRが溜まっており、レビューが進んでいない状況です。
こちらはissueにも立てられておらず、優先度は下げて良いという認識ですので、v8.1.0のマイルストーンに回し他を優先するということでよいでしょうか。

@200km 200km added this to the Minor update v8.1.0 milestone Feb 20, 2024
@sksat
Copy link
Collaborator Author

sksat commented Feb 20, 2024

@200km ありがとうございます.これは互換性のある Logger class 内部のみに収まるリファクタなので,それで構わないです.

@200km 200km added the automation::comment-graph comment simulation result graph on pull request label Jun 29, 2024
Copy link
Member

@200km 200km left a comment

Choose a reason for hiding this comment

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

こちらもレビューが遅くなり申し訳ございません。Approveしましたが、コンフリクトの解除をお願いします。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automation::comment-graph comment simulation result graph on pull request library library minor update add functionality in a backwards compatible manner priority::medium priority medium
Projects
Status: 🏗 In Progress
Development

Successfully merging this pull request may close these issues.

None yet

2 participants