-
Notifications
You must be signed in to change notification settings - Fork 56
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
[ffigen] Add config ignore-source-errors
#810
[ffigen] Add config ignore-source-errors
#810
Conversation
@dcharkes this should be ready for review. Regarding the failing windows test, since we are calling I can run the tests twice and add some logs to figure out which test will fail. But to avoid these situations, is there any sort of work around for this? Can we maybe replace the calls to |
Package publishing
Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mannprerak2!
@@ -1,3 +1,10 @@ | |||
## 10.1.0 | |||
|
|||
- Any compiler errors/warnings in source header files will now result in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What kind of warnings result in wrong generated code? Should we refuse to generate on all warnings?
If we refuse to generate for too many warnings, people might just start using ignore-source-errors
instead of fixing the issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, for now I've only handled warnings/errors from libclang. (Excluding ObjC for now)
But I think anything which can potentially generate invalid bindings, that might silently break at runtime should be covered.
Update data.dart
* Add errors.md * refer to errors.md * Update errors.md * Update errors.md * Update errors.md
* Rename pkgs/ffigen/errors.md to pkgs/ffigen/docs/errors.md * Update parser.dart
Rename pkgs/ffigen/errors.md to pkgs/ffigen/doc/errors.md
```yaml | ||
compiler-opts: | ||
- "-I/path/to/folder" | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we mention -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/
?
@liamappelbe what are typical errors we see with Objective-C?
* Update errors.md * Update parser.dart * Update errors.md
* Update CHANGELOG.md * Update pubspec.yaml
auto label is removed for dart-lang/native/810, due to This PR has not met approval requirements for merging. You are not a member of dart-team and need 1 more review(s) in order to merge this PR.
|
Closes #439
ignore-source-errors
and flag--ignore-source-errors
to prevent generation of errors.Contribution guidelines:
dart format
.Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.