Skip to content

Conversation

@svij-sc
Copy link
Collaborator

@svij-sc svij-sc commented Jan 22, 2026

Scope of work done

Without this change dep_vars.env is not available in the released wheel, as such anything that relies on gigl/common/constants.py does not work since that requires dep_vars.env being avaialble.

We move the file under the gigl dir and also make all yaml files in these dirs also package in the wheels to ensure the build mirrors the source layout and support bootstrapping.

Where is the documentation for this feature?: N/A

Did you add automated tests or write a test plan?

Updated Changelog.md? NO

Ready for code review?: YES

Base automatically changed from svij/remove-python-folder to main January 23, 2026 00:59
@svij-sc svij-sc changed the title [WIIP] move dep-vars - make wheel available for release Move dep-vars - make wheel available for release Jan 29, 2026
Copy link
Collaborator

@kmontemayor2-sc kmontemayor2-sc left a comment

Choose a reason for hiding this comment

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

Hmmm I'm not thrilled about having the scala binaries having some dependency on the python src dirs, seems "incorrect" somehow...

I guess the alternative here is to duplicate dep_vars.env directly in constants.py when we build it? tbch I sort of prefer that so the values are obvious when clicking around python src dirs (instead of needing to then go find the dep_vars.env file).

WDYT?

Otherwise this is fine.

@svij-sc
Copy link
Collaborator Author

svij-sc commented Jan 29, 2026

I guess the alternative here is to duplicate dep_vars.env directly in constants.py when we build it? tbch I sort of prefer that so the values are obvious when clicking around python src dirs (instead of needing to then go find the dep_vars.env file).

The build not mirroring src seems worse to me.

@kmontemayor2-sc
Copy link
Collaborator

It would mirror src, I'm suggesting that our scripts which generate dep_vars.env also update constants.py, but I understand if you don't want to have the de-dupe.

Copy link
Collaborator

@kmontemayor2-sc kmontemayor2-sc left a comment

Choose a reason for hiding this comment

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

Approval pending we've check the release paths work.

@svij-sc
Copy link
Collaborator Author

svij-sc commented Jan 29, 2026

@kmontemayor2-sc

Approval pending we've check the release paths work.

Yep, tested here: https://github.com/Snapchat/GiGL/actions/runs/21468608715, and publish wheel here: https://github.com/Snapchat/GiGL/actions/runs/21469349436

@svij-sc svij-sc marked this pull request as ready for review January 29, 2026 19:44
@svij-sc svij-sc added this pull request to the merge queue Jan 29, 2026
Merged via the queue into main with commit 31e17bb Jan 29, 2026
9 checks passed
@svij-sc svij-sc deleted the svij/make-whl-avail-for-release branch January 29, 2026 21:10
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.

5 participants