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: [Makefile] command is echoed out to stdout #14

Merged
merged 1 commit into from
Mar 27, 2024

Conversation

snapdgn
Copy link

@snapdgn snapdgn commented Mar 20, 2024

When running make json, it outputs a valid json without any debug outputs , EXCEPT it echoes out the command too, thus messing up the json representation.

Refer the Screenshot below for clarity.

image

This PR fixes it so that no hand-editing is required.

P.S - This was such a small change, that I didn't think creating a new branch was necessary. Let me know if I should do that in future.

Thanks

@snapdgn snapdgn changed the base branch from main to JSON March 20, 2024 19:22
Copy link
Owner

@ThinkOpenly ThinkOpenly left a comment

Choose a reason for hiding this comment

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

Actually, Nitish, could you slightly change your commit with a commit message that describes the action being taken by the commit. Something like "[JSON] Suppress command echo to stdout", and then in the commit message, could you state the current situation, why it's a problem, and what you did to fix it?

@snapdgn snapdgn force-pushed the JSON branch 4 times, most recently from 759bc97 to ad0ebb8 Compare March 22, 2024 10:24
@snapdgn
Copy link
Author

snapdgn commented Mar 24, 2024

@ThinkOpenly , Sorry, I forgot to ping you. Can this be closed now?

@ThinkOpenly
Copy link
Owner

Can this be closed now?

I'm being pedantic, but let's follow "generally accepted practices" for some important open source projects and limit the width of the commit message to less than 80 characters. Could you break the final line of your commit message before 80 characters? Thanks!

Currently, when executing `make json`, the command itself is echoed,
causing issues with the JSON representation.

This PR Suppress this echoing, ensuring a proper JSON output
without the need for any manual editing later on.
@snapdgn
Copy link
Author

snapdgn commented Mar 26, 2024

Can this be closed now?

I'm being pedantic, but let's follow "generally accepted practices" for some important open source projects and limit the width of the commit message to less than 80 characters. Could you break the final line of your commit message before 80 characters? Thanks!

Sorry, I had wrapping turned on in my vim configs at 80 chars, I don't know why this happened, maybe a misconfiguration.
Will keep this in mind from next time.
Let me know if this looks good now.

@ThinkOpenly ThinkOpenly merged commit aa8d4e3 into ThinkOpenly:JSON Mar 27, 2024
1 check passed
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.

2 participants