-
Notifications
You must be signed in to change notification settings - Fork 0
20001: docs: Automatically update DataFusion version in docs #208
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -48,7 +48,6 @@ | |||||||||||||||||||||||||
| 'datafusion-benchmarks': 'benchmarks/Cargo.toml', | ||||||||||||||||||||||||||
| 'datafusion-cli': 'datafusion-cli/Cargo.toml', | ||||||||||||||||||||||||||
| 'datafusion-examples': 'datafusion-examples/Cargo.toml', | ||||||||||||||||||||||||||
| 'datafusion-docs': 'docs/Cargo.toml', | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def update_workspace_version(new_version: str): | ||||||||||||||||||||||||||
|
|
@@ -116,7 +115,8 @@ def update_docs(path: str, new_version: str): | |||||||||||||||||||||||||
| with open(path, 'r+') as fd: | ||||||||||||||||||||||||||
| content = fd.read() | ||||||||||||||||||||||||||
| fd.seek(0) | ||||||||||||||||||||||||||
| content = re.sub(r'datafusion = "(.+)"', f'datafusion = "{new_version}"', content) | ||||||||||||||||||||||||||
| content = re.sub(r'datafusion = "(.+?)"', f'datafusion = "{new_version}"', content) | ||||||||||||||||||||||||||
| content = re.sub(r'datafusion = { version = "(.+?)"', f'datafusion = {{ version = "{new_version}"', content) | ||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing file truncation causes file corruptionHigh Severity The Additional Locations (1)
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. value:useful; category:bug; feedback:The Bugbot AI reviewer is correct! The file should be truncated before or after writing the new content. Otherwise it may leave some extra characters at the end from the old content. |
||||||||||||||||||||||||||
| fd.write(content) | ||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. value:useful; category:bug; feedback:The Augment AI reviewer is correct! The file should be truncated before or after writing the new content. Otherwise it may leave some extra characters at the end from the old content. |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
@@ -144,6 +144,9 @@ def main(): | |||||||||||||||||||||||||
| update_downstream_versions(cargo_toml, new_version) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| update_docs("README.md", new_version) | ||||||||||||||||||||||||||
| update_docs("docs/source/download.md", new_version) | ||||||||||||||||||||||||||
| update_docs("docs/source/user-guide/example-usage.md", new_version) | ||||||||||||||||||||||||||
| update_docs("docs/source/user-guide/crate-configuration.md", new_version) | ||||||||||||||||||||||||||
|
Comment on lines
146
to
+149
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The repeated calls to
Suggested change
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. value:good-to-have; category:bug; feedback:The Gemini AI reviewer is correct! Using a list of file names and iterating over them in a loop to update each of them would be more easier to read and maintain! |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if __name__ == "__main__": | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The two
re.subcalls can be combined into a single, more robust call. This new regex will handle optional whitespace and both version specification formats (datafusion = "..."anddatafusion = { version = "..." }). Using a lambda function for the replacement makes the code clearer and avoids potential escaping issues. This simplifies the code and makes it more resilient to formatting variations.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
value:good-but-wont-fix; category:bug; feedback:The Gemini AI reviewer is correct that the same could be achieved with a single regex pattern but it will be more complex to read and maintain! The regex is used is a script that is executed once per release, so it is not important to be very optimized.