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

[#1140] adds warning when the target has no target properties #3595

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

zyberzebra
Copy link

@zyberzebra zyberzebra commented May 6, 2024

Issue Reference: #1140
TODO:

  • should no accessible properties also lead to the warning?
    • I guess so
  • remove reportErrorForUnmappedTargetPropertiesIfRequired, I guess?
    • realized unmappedTarget is a different case.
  • Any other possibilities or edge cases to test?

@zyberzebra zyberzebra marked this pull request as draft May 6, 2024 19:16
- warning that no actual target properties are targeted/written to should be given when compiling
@zyberzebra zyberzebra marked this pull request as ready for review May 7, 2024 16:45
Copy link
Member

@filiphr filiphr left a comment

Choose a reason for hiding this comment

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

Thanks for your work on this @zyberzebra. I've left some comments.

@zyberzebra
Copy link
Author

@filiphr I'm not 100% what the correct workflow is. Should I request reviews when I'm ready or not?

@filiphr
Copy link
Member

filiphr commented May 27, 2024

The workflow is me trying to get time and going through all the emails and updated PRs 😀. I usually go through PRs that have been updated as well

@zyberzebra
Copy link
Author

The workflow is me trying to get time and going through all the emails and updated PRs 😀. I usually go through PRs that have been updated as well

Thanks for the clarification ! I just scrolled through some other pull request here and saw that some request reviews and because it's my first time working open source I was unsure. ;)
At work I just ping my mates 😁

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.

None yet

2 participants