search

Cytnx-dev / Cytnx

Project Cytnx, A Cross-section of Python & C++,Tensor network library
Apache License 2.0
35 stars 13 forks source link

Change header guard to use #pragma once #470

Open kaihsin opened 1 week ago

IvanaGyro commented 1 week ago

There are some decisions from the coding style guides.

To summarise, prefer header guards to#pragma once. The former is supported by the standard and is more portable.

kaihsin commented 1 week ago

Are we following Google coding style now?

On Sat, Sep 14, 2024, 13:55 Ivana @.***> wrote:

There are some decisions from the coding style guide.

To summarise, prefer header guards than #pragma once. The former is supported by the standard and is more portable.

— Reply to this email directly, view it on GitHub https://github.com/Cytnx-dev/Cytnx/issues/470#issuecomment-2351084231, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFCX3SOZISHJ3FRHZMJYZM3ZWR2BXAVCNFSM6AAAAABOGJ4JROVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJRGA4DIMRTGE . You are receiving this because you authored the thread.Message ID: @.***>

kaihsin commented 1 week ago

If so I would like to have a pre-commit tool in place for this. If not, lets keep it with c++ std guide recommentation up to the version we are targeting as long as it comply with clang-format. Other than that I don't think we should focus on those fomat too much

On Sat, Sep 14, 2024, 15:51 Kai-Hsin Wu @.***> wrote:

Are we following Google coding style now?

On Sat, Sep 14, 2024, 13:55 Ivana @.***> wrote:

There are some decisions from the coding style guide.

To summarise, prefer header guards than #pragma once. The former is supported by the standard and is more portable.

— Reply to this email directly, view it on GitHub https://github.com/Cytnx-dev/Cytnx/issues/470#issuecomment-2351084231, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFCX3SOZISHJ3FRHZMJYZM3ZWR2BXAVCNFSM6AAAAABOGJ4JROVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJRGA4DIMRTGE . You are receiving this because you authored the thread.Message ID: @.***>

IvanaGyro commented 1 week ago

Are we following Google coding style now?

It seems that, yes.

https://github.com/Cytnx-dev/Cytnx/blob/f43dcbd960b955611cddcbc036cc07e113dbbb8b/.clang-format#L1

We also have pre-commit settings

https://github.com/Cytnx-dev/Cytnx/blob/f43dcbd960b955611cddcbc036cc07e113dbbb8b/.pre-commit-config.yaml

CC @jeffry1829

kaihsin commented 1 week ago

Does it actually settled up on action? I thought its there. And it was fine before tho, so I am confused

On Sun, Sep 15, 2024, 01:22 Ivana @.***> wrote:

Are we following Google coding style now?

It seems that, yes.

https://github.com/Cytnx-dev/Cytnx/blob/f43dcbd960b955611cddcbc036cc07e113dbbb8b/.clang-format#L1

We also have pre-commit settings

https://github.com/Cytnx-dev/Cytnx/blob/f43dcbd960b955611cddcbc036cc07e113dbbb8b/.pre-commit-config.yaml

CC @jeffry1829 https://github.com/jeffry1829

— Reply to this email directly, view it on GitHub https://github.com/Cytnx-dev/Cytnx/issues/470#issuecomment-2351379982, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFCX3SJI4B7TI7XURG3HDBDZWUKRXAVCNFSM6AAAAABOGJ4JROVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJRGM3TSOJYGI . You are receiving this because you authored the thread.Message ID: @.***>

kaihsin commented 1 week ago

If it's optional requirements for format I'd say we don't worry about it and follow as much as std. Google style is good but it has too much historical burdens

On Sun, Sep 15, 2024, 01:28 Kai-Hsin Wu @.***> wrote:

Does it actually settled up on action? I thought its there. And it was fine before tho, so I am confused

On Sun, Sep 15, 2024, 01:22 Ivana @.***> wrote:

Are we following Google coding style now?

It seems that, yes.

https://github.com/Cytnx-dev/Cytnx/blob/f43dcbd960b955611cddcbc036cc07e113dbbb8b/.clang-format#L1

We also have pre-commit settings

https://github.com/Cytnx-dev/Cytnx/blob/f43dcbd960b955611cddcbc036cc07e113dbbb8b/.pre-commit-config.yaml

CC @jeffry1829 https://github.com/jeffry1829

— Reply to this email directly, view it on GitHub https://github.com/Cytnx-dev/Cytnx/issues/470#issuecomment-2351379982, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFCX3SJI4B7TI7XURG3HDBDZWUKRXAVCNFSM6AAAAABOGJ4JROVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJRGM3TSOJYGI . You are receiving this because you authored the thread.Message ID: @.***>

IvanaGyro commented 1 week ago

Client-side git hooks do not sync between git clients. Every client has to install the hook on their own. Here is a section of the document written by @hunghaoti

  1. Use the pre-commit tool to check the coding style:

    (1). If you have not installed the pre-commit tool, you can install it by the command:

    conda install pre-commit

    (2). Then you can run the following command to fix the coding style:

    pre-commit run --all-files

    You can run twice to make sure all of them are passed.

kaihsin commented 1 week ago

Yes I know. We set up this. My point is if there is no issue with clang formatter then I don't think we should care about the format even it stated in Google style. In general as long as there is a guard in action to PR there (which we setup in action) beyond that norm I don't want to put focus on making style happy. We should focus on other more important issues

On Sun, Sep 15, 2024, 01:50 Ivana @.***> wrote:

Client-side git hooks do not sync between git clients. Every client has to install the hook on their own. Here is a section of the document written by @hunghaoti https://github.com/hunghaoti

2.

Use the pre-commit tool to check the coding style:

(1). If you have not installed the pre-commit tool, you can install it by the command:

conda install pre-commit

(2). Then you can run the following command to fix the coding style:

pre-commit run --all-files

You can run twice to make sure all of them are passed.

— Reply to this email directly, view it on GitHub https://github.com/Cytnx-dev/Cytnx/issues/470#issuecomment-2351389967, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFCX3SNTNNHQRWCRYAOW7P3ZWUNZDAVCNFSM6AAAAABOGJ4JROVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJRGM4DSOJWG4 . You are receiving this because you authored the thread.Message ID: @.***>

kaihsin commented 1 week ago

Feel free to close this issue if you think we are going to stick with Google style.

On Sun, Sep 15, 2024, 01:54 Kai-Hsin Wu @.***> wrote:

Yes I know. We set up this. My point is if there is no issue with clang formatter then I don't think we should care about the format even it stated in Google style. In general as long as there is a guard there (which we setup in action) beyond that norm I don't want to put focus on making style happy. We should focus on other more important issues

On Sun, Sep 15, 2024, 01:50 Ivana @.***> wrote:

Client-side git hooks do not sync between git clients. Every client has to install the hook on their own. Here is a section of the document written by @hunghaoti https://github.com/hunghaoti

2.

Use the pre-commit tool to check the coding style:

(1). If you have not installed the pre-commit tool, you can install it by the command:

conda install pre-commit

(2). Then you can run the following command to fix the coding style:

pre-commit run --all-files

You can run twice to make sure all of them are passed.

— Reply to this email directly, view it on GitHub https://github.com/Cytnx-dev/Cytnx/issues/470#issuecomment-2351389967, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFCX3SNTNNHQRWCRYAOW7P3ZWUNZDAVCNFSM6AAAAABOGJ4JROVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJRGM4DSOJWG4 . You are receiving this because you authored the thread.Message ID: @.***>