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

Move the source of truth of kaggle-api development to this repository #557

Merged
merged 26 commits into from
Mar 26, 2024

Conversation

Philmod
Copy link
Contributor

@Philmod Philmod commented Mar 25, 2024

Changes

  • It uses hatch, for example hatch run compile to generate the python files to release to pypi
  • The generated code is checked-in so that users can pip install from source, as before this change
  • The important files to review are:
    • the new README.md
    • the tools/GeneratePythonLibrary.sh that was modified

Next

  • Remove code from main kaggle repo
  • Internal documentation for release, etc...

Ideas for medium-term

Bug

http://b/325046928

Copy link
Contributor

@erdalsivri erdalsivri left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@stevemessick stevemessick left a comment

Choose a reason for hiding this comment

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

I made lots of comments about line breaks. Those can be considered optional. I did clean up the language in a few places and those should be included.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
docs/README.md Show resolved Hide resolved
docs/README.md Outdated Show resolved Hide resolved
docs/README.md Show resolved Hide resolved
docs/README.md Show resolved Hide resolved
docs/README.md Show resolved Hide resolved
docs/README.md Show resolved Hide resolved
docs/README.md Show resolved Hide resolved
docs/README.md Show resolved Hide resolved
@stevemessick
Copy link
Contributor

stevemessick commented Mar 25, 2024

Also, note that when you first see my comments you may have a bunch that are collapsed. I had to click the line that said there were 26 hidden comments to see them all.

Edit: even then, there were still six collapsed under another See more... block.

Copy link
Member

@neshdev neshdev left a comment

Choose a reason for hiding this comment

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

LGTM.

I would have assume that the delta between the repos to be a lot smaller. Has the repo not been synced for a while?

Also, is it possible to get ride of the formatting changes?

@Philmod
Copy link
Contributor Author

Philmod commented Mar 26, 2024

LGTM.

I would have assume that the delta between the repos to be a lot smaller. Has the repo not been synced for a while?

Also, is it possible to get ride of the formatting changes?

The diff in the kaggle/ directory are indeed mostly formatting changes, but unclear where they come from.

@Philmod Philmod merged commit 8891e72 into main Mar 26, 2024
2 checks passed
@Philmod Philmod deleted the source-of-truth branch March 26, 2024 11:52
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