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

Revert "build: upgrade clang-format to v18" #54994

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

legendecas
Copy link
Member

@legendecas legendecas commented Sep 18, 2024

This reverts commit c3e1c31.

@wasm-fmt/clang-format outputs incomplete env.cc (81K) or node_contextify.cc (69K) with any changes in it.

Refs: #54993 (comment)
Refs: #53957

@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Sep 18, 2024
@RedYetiDev
Copy link
Member

😅

Sorry! I should've checked the output of a complete format before allowing the PR to be merged.

@RedYetiDev RedYetiDev added c++ Issues and PRs that require attention from people who are familiar with C++. revert PRs that revert previously landed PRs. labels Sep 18, 2024
@richardlau richardlau added the fast-track PRs that do not need to wait for 48 hours to land. label Sep 18, 2024
@RedYetiDev
Copy link
Member

CC @magic-akari

Copy link
Contributor

Fast-track has been requested by @richardlau. Please 👍 to approve.

@richardlau richardlau added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 18, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 18, 2024
@nodejs-github-bot nodejs-github-bot merged commit 7e00be7 into nodejs:main Sep 18, 2024
34 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 7e00be7

@joyeecheung
Copy link
Member

Somehow after this revert, I am still seeing this strange cut-off env.cc thing both in the Github actions (https://github.com/nodejs/node/actions/runs/10900721511/job/30436607404?pr=54971) and locally for #54971 - I tried to rebase and re-ran make format-cpp-build locally, but it is still showing up.

@joyeecheung
Copy link
Member

Managed to fix it locally by running make format-cpp-clean

@magic-akari
Copy link

My apologies for the inconvenience I've caused.
I should have conducted more thorough tests before publishing.

@RedYetiDev
Copy link
Member

No worries! Alls well that ends well.

targos pushed a commit that referenced this pull request Oct 4, 2024
This reverts commit c3e1c31.

PR-URL: #54994
Refs: #53957
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@aduh95 aduh95 mentioned this pull request Oct 9, 2024
louwers pushed a commit to louwers/node that referenced this pull request Nov 2, 2024
This reverts commit c3e1c31.

PR-URL: nodejs#54994
Refs: nodejs#53957
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. fast-track PRs that do not need to wait for 48 hours to land. revert PRs that revert previously landed PRs. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants