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

Revert tzdata update and update lock #1238

Merged
merged 8 commits into from
Apr 19, 2024
Merged

Conversation

sultaniman
Copy link
Contributor

This PR reverts tzdata update in pyproject.toml and updates lockfile.

@sultaniman sultaniman self-assigned this Apr 17, 2024
@sultaniman sultaniman marked this pull request as ready for review April 17, 2024 19:35
Copy link

netlify bot commented Apr 17, 2024

Deploy Preview for dlt-hub-docs canceled.

Name Link
🔨 Latest commit cc799bf
🔍 Latest deploy log https://app.netlify.com/sites/dlt-hub-docs/deploys/662141de508c8b0008975b5b

Copy link
Collaborator

@rudolfix rudolfix left a comment

Choose a reason for hiding this comment

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

@sultaniman please add

  1. new label "changes dependencies" that we will add to PRs that change deps
  2. explanation to CONTRIBUTING:
### Add / update / remove core library dependencies

where you'll explain the process and why we do it!

CONTRIBUTING.md Outdated
The other important note on using possible version minimas is to prevent potential cases where package
versions will not be resolvable.

### Hypothetical example:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can scrap this section, the above section is clear enough. Also we are not as much concerned about breaking changes (I think) than about other libs our users are using which will collide with our requirements, so in this case we would not enforce a higher minimal version I think. @rudolfix wdyt.

Copy link
Collaborator

@rudolfix rudolfix left a comment

Choose a reason for hiding this comment

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

yeah now it makes sense! @sultaniman @sh-rp should we introduce a label to warns that dependencies are updated? or maybe we add this to the PR template? so you must say if you update core deps or not

@sultaniman you can drop the example

@sultaniman
Copy link
Contributor Author

@rudolfix @sh-rp I added dependency update label

Copy link
Collaborator

@rudolfix rudolfix left a comment

Choose a reason for hiding this comment

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

OK! let's merge this and change our PR template - ask about deps changes there

@rudolfix rudolfix merged commit 4750f62 into devel Apr 19, 2024
46 of 48 checks passed
@rudolfix rudolfix deleted the deps/revert-tzdata-update branch April 19, 2024 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants