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

Add Pydantic Model for pr_info, pr_json, and versions #2265

Merged
merged 14 commits into from
Apr 8, 2024

Conversation

ytausch
Copy link
Contributor

@ytausch ytausch commented Mar 14, 2024

Extending #2239, this PR adds documenting Pydantic models for the pr_info, pr_json, and versions JSON collections.

#2239 should be merged first.

@ytausch ytausch changed the title Add Pydantic Model for pr_info and pr_json Add Pydantic Model for pr_info, pr_json, and versions Mar 14, 2024
Copy link

codecov bot commented Mar 14, 2024

Codecov Report

Attention: Patch coverage is 69.23077% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 71.09%. Comparing base (7288353) to head (fc78254).
Report is 5 commits behind head on master.

❗ Current head fc78254 differs from pull request most recent head 8296c3c. Consider uploading reports for the commit 8296c3c to get more accurate results

Files Patch % Lines
conda_forge_tick/git_utils.py 33.33% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2265      +/-   ##
==========================================
- Coverage   71.44%   71.09%   -0.36%     
==========================================
  Files          99       97       -2     
  Lines        9838     9639     -199     
==========================================
- Hits         7029     6853     -176     
+ Misses       2809     2786      -23     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@beckermr beckermr left a comment

Choose a reason for hiding this comment

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

Please remove all functional changes from this PR. We are more than happy to accept them in other PRs, but it makes it hard to read the diff. Let's only do typing and pydantic models here.

@ytausch ytausch force-pushed the pydantic_prinfo branch 4 times, most recently from 047bf1e to 8091476 Compare March 18, 2024 18:07
@ytausch ytausch marked this pull request as ready for review April 5, 2024 17:46
@ytausch ytausch marked this pull request as draft April 5, 2024 17:46
@ytausch ytausch marked this pull request as ready for review April 5, 2024 17:51
@ytausch
Copy link
Contributor Author

ytausch commented Apr 5, 2024

@beckermr
It's been a while since I last worked on this - I don't see any functional changes anymore and I also remember opening a separate PR for them, feel free to tell me if there are any left

Also, any clue why pydantic is not found in the CI? It's part of the environment

@beckermr
Copy link
Contributor

beckermr commented Apr 5, 2024

you need to regenerate the lock file I think.

Copy link
Contributor

@beckermr beckermr left a comment

Choose a reason for hiding this comment

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

Some questions.

conda_forge_tick/models/common.py Outdated Show resolved Hide resolved
conda_forge_tick/models/pr_info.py Show resolved Hide resolved
@ytausch
Copy link
Contributor Author

ytausch commented Apr 7, 2024

Why does regenerating the lock file with conda-lock change everything about the lock file (including key ordering)? This seems a bit weird. Maybe the current lock file was generated with an outdated version of coda-lock? I used 2.5.6.

@ytausch ytausch requested a review from beckermr April 7, 2024 18:03
@beckermr
Copy link
Contributor

beckermr commented Apr 7, 2024

Yeah it is annoying. It turns out conda-lock itself doesn't produce lock files with nice diffs. I ended up writing some custom code in

https://github.com/regro/cf-scripts/blob/master/autotick-bot/relock_me.py

that produces stable, sorted lock files that have much smaller diffs.

I'm ok merging the changes here, but we could run the latest lock file through the function in that script to make them cleaner.

I should upstream that bit of code to conda lock, but have not had the time.

If you want to do that, great, but no pressure from me. I know you are busy.

@beckermr
Copy link
Contributor

beckermr commented Apr 8, 2024

Closing and reopening to cycle the ci

@beckermr beckermr closed this Apr 8, 2024
@beckermr beckermr reopened this Apr 8, 2024
@ytausch
Copy link
Contributor Author

ytausch commented Apr 8, 2024

I think the CI is not working because of the lock file conflicts.

@beckermr
Copy link
Contributor

beckermr commented Apr 8, 2024

pre-commit.ci autofix

@beckermr
Copy link
Contributor

beckermr commented Apr 8, 2024

@ytausch one more issue to be resolved on smithy version validation and then this one is good to go.

@beckermr beckermr merged commit ced9e43 into regro:master Apr 8, 2024
2 checks passed
@ytausch ytausch mentioned this pull request Apr 9, 2024
1 task
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