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

[make] full implementation according specification #160

Merged
merged 1 commit into from
Oct 26, 2024

Conversation

Wandalen
Copy link
Contributor

@Wandalen Wandalen commented Jul 5, 2024

No description provided.

@jgarzik jgarzik added the enhancement New feature or request label Jul 11, 2024
@Wandalen Wandalen force-pushed the make branch 3 times, most recently from 348f359 to a7e977e Compare July 19, 2024 16:49
@Wandalen Wandalen marked this pull request as ready for review July 19, 2024 17:00
@Wandalen Wandalen changed the title [make] first implementation [make] basic implementation Jul 19, 2024
@Wandalen Wandalen marked this pull request as draft September 13, 2024 20:32
@Wandalen Wandalen force-pushed the make branch 3 times, most recently from 6d45c76 to a347ee7 Compare October 19, 2024 07:04
@Wandalen Wandalen changed the title [make] basic implementation [make] almost ready Oct 21, 2024
@Wandalen Wandalen force-pushed the make branch 3 times, most recently from 2cd17dd to 6c7862c Compare October 23, 2024 05:43
@Wandalen Wandalen marked this pull request as ready for review October 23, 2024 06:05
@Wandalen Wandalen changed the title [make] almost ready [make] full implementation according specification Oct 23, 2024
@Wandalen
Copy link
Contributor Author

@jgarzik ready for review

Copy link
Contributor

@jgarzik jgarzik left a comment

Choose a reason for hiding this comment

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

Mostly looks OK. Some minor changes.

@@ -109,7 +109,7 @@ fn run_command(input: &Path) -> std::process::Output {
#[test]
fn test_bsd() {
init();
let output = run_command(Path::new("fixtures/integration_tests/bsd.m4"));
let output = run_command(&Path::new("fixtures/integration_tests/bsd.m4"));
Copy link
Contributor

Choose a reason for hiding this comment

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

merge error. This PR should not touch m4/

@@ -84,9 +84,6 @@ pub fn run_test(plan: TestPlan) {
let stdout = String::from_utf8_lossy(&output.stdout);
assert_eq!(stdout, plan.expected_out);

let stderr = String::from_utf8_lossy(&output.stderr);
assert_eq!(stderr, plan.expected_err);

Copy link
Contributor

Choose a reason for hiding this comment

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

Change rejected -- This change deletes some tests across all utilities.

@Wandalen Wandalen marked this pull request as draft October 23, 2024 14:31
@Wandalen Wandalen marked this pull request as ready for review October 24, 2024 08:01
@Wandalen
Copy link
Contributor Author

@jgarzik took into account. is there anything?

@jgarzik jgarzik merged commit 00c7c6d into rustcoreutils:main Oct 26, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants