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

Cd to repo root before paramalarm #1805

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

Conversation

knoellle
Copy link
Contributor

Why? What?

What it says. previously crashed because it was unable to find the json files.

Fixes #

ToDo / Known Issues

If this is a WIP describe which problems are to be fixed.

Ideas for Next Iterations (Not This PR)

If there are some improvements that could be done in a next iteration, describe them here.

How to Test

Cd somewhere other than repo root and run parameter tester

@knoellle knoellle added tools:Tooling Related to pepsi et.al. GO25 labels Mar 16, 2025
@knoellle knoellle enabled auto-merge March 16, 2025 11:58
@knoellle knoellle force-pushed the position-independent-paramalarm branch from 4741ccf to 0c666eb Compare March 16, 2025 12:06
@julianschuler julianschuler self-assigned this Mar 17, 2025
Some(path) => PathBuf::from(path),
None => PathBuf::from("."),
};
let repository = Repository::find_root(repository_search_path).expect("no repository found");
Copy link
Member

Choose a reason for hiding this comment

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

Why do use expect here instead of e.g. wrap_err and propagating the error?


let file =
File::open(framework_parameters_path).wrap_err("failed to open framework parameters")?;
let file = File::open("etc/parameters/framework.json")
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about joining this path with the repository root instead of first changing the current directory and then using a relative path?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GO25 tools:Tooling Related to pepsi et.al.
Projects
Status: Request for Review
Development

Successfully merging this pull request may close these issues.

2 participants