Skip to content

MINOR: Fix formatting in parquet.thrift file (trailing spaces)#558

Merged
alamb merged 1 commit intoapache:masterfrom
OmBiradar:parquet.thrift-formatting-error
Mar 19, 2026
Merged

MINOR: Fix formatting in parquet.thrift file (trailing spaces)#558
alamb merged 1 commit intoapache:masterfrom
OmBiradar:parquet.thrift-formatting-error

Conversation

@OmBiradar
Copy link
Copy Markdown
Contributor

@OmBiradar OmBiradar commented Mar 12, 2026

There were many trailing spaces here and there,
they have been removed.

Rationale for this change

Used the zed editors linter to format the document - removing any trailing spaces.

What changes are included in this PR?

Just removed the trailing spaces.

There were many trailing spaces here and there,
they have been removed.

Signed-off-by: OmBiradar <ombiradar04@gmail.com>
@OmBiradar OmBiradar changed the title Fix formatting in parquet.thrift file (trailing spaces) MINOR: Fix formatting in parquet.thrift file (trailing spaces) Mar 12, 2026
@OmBiradar
Copy link
Copy Markdown
Contributor Author

original issue - apache-arrow-issue-49501

requesting your review @alamb

Copy link
Copy Markdown
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

THis is a nice cleanup in my opinion

Thank you @OmBiradar

@alamb
Copy link
Copy Markdown
Contributor

alamb commented Mar 12, 2026

(I'll leave this open for a while to make sure other committers have a chance to review if they would like)

Copy link
Copy Markdown
Member

@wgtmac wgtmac left a comment

Choose a reason for hiding this comment

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

+1 from me. I know the intention is good but I don't know if anyone really cares about the original commit history.

@OmBiradar
Copy link
Copy Markdown
Contributor Author

OmBiradar commented Mar 12, 2026

@alamb @wgtmac

Should I add a new CI task to check the linting in this PR itself?

@alamb
Copy link
Copy Markdown
Contributor

alamb commented Mar 12, 2026

@alamb @wgtmac

Should I add a new CI task to check the linting in this PR itself?

No, if you want to propose a new CI please use a new PR. I am not sure we are likely to add new CI 🤔

@alamb
Copy link
Copy Markdown
Contributor

alamb commented Mar 12, 2026

+1 from me. I know the intention is good but I don't know if anyone really cares about the original commit history.

The commit history will be retained, but just using git blame directly will be harder to interpret (will require more archeology)

@OmBiradar
Copy link
Copy Markdown
Contributor Author

What if we add a new '.git-blame-ignore-revs' file that will help avoid these formatting/linting related commits to interfear with the 'git blame'

https://www.git-tower.com/blog/how-to-exclude-commits-from-git-blame#:~:text=You%20can%20do%20this%20by,ignoreRevsFile%20configuration%20option.&text=With%20this%20configuration%20in%20place,blame%2Dignore%2Drevs%20file.

@pitrou
Copy link
Copy Markdown
Member

pitrou commented Mar 18, 2026

The commit history will be retained, but just using git blame directly will be harder to interpret (will require more archeology)

git blame -w is supposed to ignore whitespace changes like these.

Copy link
Copy Markdown
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thanks everyone. I double checked we can still do git archeology reasonably using git blame -w (thanks @pitrou for the hint)

1: optional binary max;
2: optional binary min;
/**
/**
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

using git blame -w does indeed correctly ignore whitespace differences:

git blame -w src/main/thrift/parquet.thrift
...
2c4ada8e src/thrift/parquet.thrift      (julien                2013-09-17 18:21:15 -0700  267) struct Statistics {
041708da src/main/thrift/parquet.thrift (Ryan Blue             2017-04-17 11:23:41 -0700  268)    /**
041708da src/main/thrift/parquet.thrift (Ryan Blue             2017-04-17 11:23:41 -0700  269)     * DEPRECATED: min and max value of the column. Use min_value and max_value.
041708da src/main/thrift/parquet.thrift (Ryan Blue             2017-04-17 11:23:41 -0700  270)     *
041708da src/main/thrift/parquet.thrift (Ryan Blue             2017-04-17 11:23:41 -0700  271)     * Values are encoded using PLAIN encoding, except that variable-length byte
041708da src/main/thrift/parquet.thrift (Ryan Blue             2017-04-17 11:23:41 -0700  272)     * arrays do not include a length prefix.
041708da src/main/thrift/parquet.thrift (Ryan Blue             2017-04-17 11:23:41 -0700  273)     *
bef54389 src/main/thrift/parquet.thrift (Zoltan Ivanfi         2017-10-06 16:38:53 -0700  274)     * These fields encode min and max values determined by signed comparison
041708da src/main/thrift/parquet.thrift (Ryan Blue             2017-04-17 11:23:41 -0700  275)     * only. New files should use the correct order for a column's logical type
041708da src/main/thrift/parquet.thrift (Ryan Blue             2017-04-17 11:23:41 -0700  276)     * and store the values in the min_value and max_value fields.
041708da src/main/thrift/parquet.thrift (Ryan Blue             2017-04-17 11:23:41 -0700  277)     *
041708da src/main/thrift/parquet.thrift (Ryan Blue             2017-04-17 11:23:41 -0700  278)     * To support older readers, these may be set when the column order is
bef54389 src/main/thrift/parquet.thrift (Zoltan Ivanfi         2017-10-06 16:38:53 -0700  279)     * signed.
041708da src/main/thrift/parquet.thrift (Ryan Blue             2017-04-17 11:23:41 -0700  280)     */
2c4ada8e src/thrift/parquet.thrift      (julien                2013-09-17 18:21:15 -0700  281)    1: optional binary max;
2c4ada8e src/thrift/parquet.thrift      (julien                2013-09-17 18:21:15 -0700  282)    2: optional binary min;
db687874 src/main/thrift/parquet.thrift (mwish                 2024-08-23 15:30:20 +0800  283)    /**
db687874 src/main/thrift/parquet.thrift (mwish                 2024-08-23 15:30:20 +0800  284)     * Count of null values in the column.

@alamb alamb merged commit 285b6fd into apache:master Mar 19, 2026
4 checks passed
@alamb
Copy link
Copy Markdown
Contributor

alamb commented Mar 19, 2026

Thanks again for the help cleaning this up @OmBiradar

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.

4 participants