Skip to content

Conversation

kevinjqliu
Copy link
Contributor

@kevinjqliu kevinjqliu commented May 17, 2025

Which issue does this PR close?

What changes are included in this PR?

Bump library version to 0.5.0

Are these changes tested?

@kevinjqliu
Copy link
Contributor Author

One question, should we remove this function as part of the 0.5.0 release or perhaps change it to since = "0.5.0"

https://github.com/apache/iceberg-rust/blame/cfe2a980b27cda744fab8e78b4550b167b4d4d45/crates/iceberg/src/io/file_io.rs#L98

This was introduced in #1275

@kevinjqliu
Copy link
Contributor Author

I also have the changes for CHANGELOG.md and DEPENDENCIES.rust.tsv queued up. Should I include those in this PR or a followup PR?

@liurenjie1024
Copy link
Contributor

liurenjie1024 commented May 19, 2025

One question, should we remove this function as part of the 0.5.0 release or perhaps change it to since = "0.5.0"

https://github.com/apache/iceberg-rust/blame/cfe2a980b27cda744fab8e78b4550b167b4d4d45/crates/iceberg/src/io/file_io.rs#L98

This was introduced in #1275

It's marked as deprecated, so I think it's better to keep it for a while so that downstream users have time to migrate them?

I also have the changes for CHANGELOG.md and DEPENDENCIES.rust.tsv queued up. Should I include those in this PR or a followup PR?

This seems like a small pr, so I'm fine with including them together, but it's up to you.

liurenjie1024
liurenjie1024 previously approved these changes May 19, 2025
Copy link
Contributor

@liurenjie1024 liurenjie1024 left a comment

Choose a reason for hiding this comment

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

Thanks @kevinjqliu for this pr!

Fokko
Fokko previously approved these changes May 19, 2025
@liurenjie1024
Copy link
Contributor

cc @kevinjqliu Are we going to merge this or we should hold on a while for CHANGELOG.md and DEPENDENCIES.rust.tsv?

@kevinjqliu kevinjqliu dismissed stale reviews from Fokko and liurenjie1024 via f0556a9 May 20, 2025 05:20
@kevinjqliu kevinjqliu force-pushed the kevinjqliu/bump-version-to-0.5.0 branch from e9d0445 to f0556a9 Compare May 20, 2025 05:20
@kevinjqliu
Copy link
Contributor Author

Added CHANGELOG.md and DEPENDENCIES.rust.tsv

Feel free to merge this and i'll push the RC tag once its merge :)

Copy link
Contributor

@liurenjie1024 liurenjie1024 left a comment

Choose a reason for hiding this comment

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

Thanks @kevinjqliu !

@liurenjie1024 liurenjie1024 merged commit e488f45 into apache:main May 20, 2025
18 checks passed
@kevinjqliu kevinjqliu deleted the kevinjqliu/bump-version-to-0.5.0 branch May 20, 2025 14:54
kevinjqliu added a commit that referenced this pull request May 22, 2025
## Which issue does this PR close?

<!--
We generally require a GitHub issue to be filed for all bug fixes and
enhancements and this helps us generate change logs for our releases.
You can link an issue to this PR using the GitHub syntax. For example
`Closes #123` indicates that this PR will close issue #123.
-->

- Related to #1325.

## What changes are included in this PR?

<!--
Provide a summary of the modifications in this PR. List the main changes
such as new features, bug fixes, refactoring, or any other updates.
-->
`DEPENDENCIES.rust.tsv` files were accidentally removed as part of the
0.5.0 prep PR (#1345). It was added back and updated in the follow up PR
(#1363).

I realized this happened because I did not install `cargo-deny` in my
env and the dependency generation script did not error.

This PR changes generation script (`python3 ./scripts/dependencies.py
generate`) to error when the underlying subprocess call (`cargo deny`)
errors.

I also ran `uvx ruff format scripts/dependencies.py` to format the
script

## Are these changes tested?

<!--
Specify what test covers (unit test, integration test, etc.).

If tests are not included in your PR, please explain why (for example,
are they covered by existing tests)?
-->
Yes, manually

I was also able to install `cargo-deny` and confirm #1363 changes are
all correct
```
cargo install --locked cargo-deny
```
kevinjqliu added a commit that referenced this pull request May 22, 2025
## Which issue does this PR close?

<!--
We generally require a GitHub issue to be filed for all bug fixes and
enhancements and this helps us generate change logs for our releases.
You can link an issue to this PR using the GitHub syntax. For example
`Closes #123` indicates that this PR will close issue #123.
-->

- Closes #.

## What changes are included in this PR?
Adds `python3 ./scripts/dependencies.py check` to the release script.
This will check dependencies' licenses. Helps prevent issue like #1345
where the `DEPENDENCIES.rust.tsv` files were accidentally removed

Also moved apache license check to the beginning of the release script

<!--
Provide a summary of the modifications in this PR. List the main changes
such as new features, bug fixes, refactoring, or any other updates.
-->

## Are these changes tested?

<!--
Specify what test covers (unit test, integration test, etc.).

If tests are not included in your PR, please explain why (for example,
are they covered by existing tests)?
-->

Yes, verified by running
```
ICEBERG_VERSION=0.5.0 ICEBERG_VERSION_RC=2 ./scripts/release.sh
```
@kevinjqliu
Copy link
Contributor Author

thanks for the review @liurenjie1024

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.

3 participants