Skip to content

Conversation

vitorsouzaalmeida
Copy link
Contributor

Fix #4093
Prevent publishing packages with empty modules and emit warnings during compilation when modules contain no public definitions.

@vitorsouzaalmeida vitorsouzaalmeida marked this pull request as draft July 21, 2025 12:50
@vitorsouzaalmeida vitorsouzaalmeida marked this pull request as ready for review July 21, 2025 12:55
@vitorsouzaalmeida
Copy link
Contributor Author

I'm not sure about this test-wasm CI error, but checking for passed PRs like #4791 I think it may be something related to my changes so I will check it

@vitorsouzaalmeida
Copy link
Contributor Author

vitorsouzaalmeida commented Jul 21, 2025

BTW, a good thing would be to add some files like CHANGELOG.md to not trigger all CI checks. All CI tasks are running again because I changed the CHANGELOG.md

@vitorsouzaalmeida
Copy link
Contributor Author

By checking this PR, I think I should add my changes to Unreleased?

@vitorsouzaalmeida vitorsouzaalmeida force-pushed the feat/empty-module-validation branch from 41e93ec to 45590fc Compare July 21, 2025 13:39
@giacomocavalieri
Copy link
Member

By checking this PR, I think I should add my changes to Unreleased?

Yeah!

@vitorsouzaalmeida
Copy link
Contributor Author

Hey @giacomocavalieri
I've just moved my changelog to file bottom and it's already under Compiler changes - which is under Unreleased.

Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Very nice! I've left some small comments inline

@vitorsouzaalmeida
Copy link
Contributor Author

I've just updated the code @lpil

Copy link
Member

@GearsDatapacks GearsDatapacks left a comment

Choose a reason for hiding this comment

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

This looks almost ready! I've left one comment, and you'll need to rebase on main to fix merge conflicts.

Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Thank you, couple small notes inline! 🙏

@vitorsouzaalmeida
Copy link
Contributor Author

Thank you guys by reviewing it again. I will back on it between today and tomorrow

Prevent publishing packages with empty modules and emit warnings during compilation when modules contain no public definitions.
Moved from AST checking to module interface, similar to what I did on publish.rs.

Also, previously the code counted public definitions from the AST and checked if the definitions list was empty. Now it directly checks if both the values and types HashMaps in the module interface are empty, as suggested by @GearsDatapacks
@vitorsouzaalmeida vitorsouzaalmeida force-pushed the feat/empty-module-validation branch from 4d6d163 to db68575 Compare August 30, 2025 20:09
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.

Prevent publishing packages with empty modules.
4 participants