Skip to content

Conversation

@gcasa
Copy link
Member

@gcasa gcasa commented Oct 11, 2025

Add new function to bring up a panel relative to a window

@gcasa gcasa requested a review from fredkiefer as a code owner October 11, 2025 22:03
Copy link
Member

@fredkiefer fredkiefer left a comment

Choose a reason for hiding this comment

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

It would be great to fix the issue I found, but then somebody already broke the code and your change isn't any worse that that.

@fredkiefer
Copy link
Member

It feels very wrong when you tell me that a PR is complete and I should review it and after the review you make significant changes that are unrelated to my comments. I should start again with a complete review but then you probably will change something else. You do understand this is the main reason I normally wait at least a month before reviewing your code?

@gcasa
Copy link
Member Author

gcasa commented Oct 13, 2025

It feels very wrong when you tell me that a PR is complete and I should review it and after the review you make significant changes that are unrelated to my comments. I should start again with a complete review but then you probably will change something else. You do understand this is the main reason I normally wait at least a month before reviewing your code?

I actually meant to mark this one as a draft. As it has a couple of issues. It won't happen again. You don't have to delay a month or so. I'll do better.

@gcasa gcasa marked this pull request as draft October 13, 2025 15:01
@gcasa
Copy link
Member Author

gcasa commented Oct 13, 2025

I will be more careful in the future. You won't have to delay your reviews. I should have marked this one as a draft so that you wouldn't review it too soon. The issue is that the panel is not coming to the front as it should and I am not sure why. I will mark this as ready when I have addressed this problem.

@gcasa
Copy link
Member Author

gcasa commented Oct 14, 2025

I may have a few very minor fixes after this. I will address the concern you have above.

@gcasa
Copy link
Member Author

gcasa commented Oct 14, 2025

I believe this PR is done @fredkiefer. No further changes.

@gcasa
Copy link
Member Author

gcasa commented Oct 14, 2025

NSRunPanelRelativeToWindow_image

@gcasa gcasa marked this pull request as ready for review October 14, 2025 02:02
@gcasa
Copy link
Member Author

gcasa commented Oct 14, 2025

I have addressed the comment and I reverted the code to close to what was originally approved with the fix I mentioned to the order of parameters. I am going to proceed with the merge.

@gcasa gcasa merged commit f1ae189 into master Oct 14, 2025
4 checks passed
@gcasa gcasa deleted the NSAlert_branch branch October 14, 2025 06:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants