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

Unvendor Archery #459

Merged
merged 1 commit into from
Jun 16, 2021
Merged

Unvendor Archery #459

merged 1 commit into from
Jun 16, 2021

Conversation

kszucs
Copy link
Member

@kszucs kszucs commented Jun 15, 2021

Which issue does this PR close?

Closes #.

Rationale for this change

Instead of vendoring Archery to the arrow-rs repository we should rather clone arrow and install it from there so future updates will be available downstream.

What changes are included in this PR?

Are there any user-facing changes?

@@ -49,78 +49,6 @@ jobs:
--event-name ${{ github.event_name }} \
--event-payload ${{ github.event_path }}

autotune:
Copy link
Member Author

@kszucs kszucs Jun 15, 2021

Choose a reason for hiding this comment

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

I don't think that this is useful for the rust implementation.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree -- I didn't even know it was there lol

- name: Set up Python
uses: actions/setup-python@v2
with:
python-version: 3.8
- name: Install Archery and Crossbow dependencies
run: pip install -e arrow/dev/archery[bot]
run: pip install -e dev/archery[bot]
Copy link
Member Author

Choose a reason for hiding this comment

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

Keeping the comment bot for the time being.

CHANGELOG.md
dev/requirements*.txt
Copy link
Member Author

Choose a reason for hiding this comment

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

These files only exist in the arrow repository.

@codecov-commenter
Copy link

Codecov Report

Merging #459 (b7dc9f6) into master (d41ca5f) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #459   +/-   ##
=======================================
  Coverage   82.64%   82.64%           
=======================================
  Files         164      164           
  Lines       45508    45508           
=======================================
+ Hits        37608    37609    +1     
+ Misses       7900     7899    -1     
Impacted Files Coverage Δ
parquet/src/encodings/encoding.rs 95.04% <0.00%> (+0.19%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d41ca5f...b7dc9f6. Read the comment docs.

Copy link
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.

I looked at the integration tests: https://github.com/apache/arrow-rs/pull/459/checks?check_run_id=2827970303

and the RAT: https://github.com/apache/arrow-rs/runs/2827970245?check_suite_focus=true

Looks like it is working well to me 👍

Thank you @kszucs

@@ -49,78 +49,6 @@ jobs:
--event-name ${{ github.event_name }} \
--event-payload ${{ github.event_path }}

autotune:
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree -- I didn't even know it was there lol

name: Lint C++, Python, R, Rust, Docker, RAT

rat:
name: Release Audit Tool (RAT)
Copy link
Contributor

Choose a reason for hiding this comment

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

Today I learned: "RAT" = "Release Audit Tool" 👍

Copy link
Member

@jorgecarleitao jorgecarleitao left a comment

Choose a reason for hiding this comment

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

Awesome! Thank you!

@jorgecarleitao jorgecarleitao merged commit ca3240a into apache:master Jun 16, 2021
@jimexist
Copy link
Member

maybe we can do the same to datafusion repo?

@alamb
Copy link
Contributor

alamb commented Jun 17, 2021

maybe we can do the same to datafusion repo?

Sounds like a good idea to me

@kszucs
Copy link
Member Author

kszucs commented Jun 17, 2021

Seems like there is plenty of stuff which we could remove from dev.

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.

None yet

5 participants