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

Declare function to silence warning #412

Merged
merged 1 commit into from
Dec 1, 2023

Conversation

matsl
Copy link
Collaborator

@matsl matsl commented Dec 1, 2023

What

Remove warning for unknown function.

Why

Keeping the number of warning as low as possible.

@matsl matsl requested a review from rswgnu December 1, 2023 22:45
Copy link
Owner

@rswgnu rswgnu left a comment

Choose a reason for hiding this comment

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

Generally ahould declar in alphabetical irder by function name for easy reference.

@matsl matsl force-pushed the remove-warning-ibtype_def-symbol branch from 61edca0 to 95f8db7 Compare December 1, 2023 23:39
@matsl matsl merged commit e032770 into master Dec 1, 2023
5 checks passed
@matsl matsl deleted the remove-warning-ibtype_def-symbol branch December 1, 2023 23:40
@matsl
Copy link
Collaborator Author

matsl commented Dec 7, 2023

@rswgnu I agree I just tossed this in here. I'm conflicted between keeping them grouped around what external package they come from and generally ordered.

All these declarations feels like a code smell to me. At least for the internal dependencies should we not strive towards just using requires? For the external dependencies I guess it is the only practical way to remove these warnings. In practice it has been a good tool for reducing even the internal warnings. All attempts with using require has failed due to cross or circular dependencies between files. We should not settle for that in the long run.

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.

2 participants