Skip to content

Conversation

topher1kenobe
Copy link
Contributor

Change
function notify_users( string $content = '' ) : string {

to
function notify_users( string|null $content = '' ) : string {

@szepeviktor
Copy link
Contributor

Is that PHP 7.4 compatible?

@topher1kenobe
Copy link
Contributor Author

Is that PHP 7.4 compatible?

Ah no, sorry. I'll adjust.

@topher1kenobe
Copy link
Contributor Author

I'm not sure why my commits aren't signed. I signed them, and saw the signage in the commit message.

@szepeviktor
Copy link
Contributor

szepeviktor commented Jun 7, 2025

The DCO bot wants you to add Signed-off-by to your commit message, not a digital signature.

@topher1kenobe
Copy link
Contributor Author

The DCO bot wants you to add Signed-off-by to your commit message, not a digital signature.

I'm aware. That didn't work and I don't know why.

@szepeviktor
Copy link
Contributor

@topher1kenobe
Copy link
Contributor Author

👉🏻 https://github.com/fairpm/fair-plugin/pull/34/checks

I read that, and followed those instructions. That's how we got here.

@Ipstenu
Copy link
Contributor

Ipstenu commented Jun 8, 2025

@topher1kenobe The problem is you have two commits that weren’t signed off. I had the same problem when I messed up signing one and the whole PR held on to it.

Did you do this part? The force push was the only way to fix it.

To add your Signed-off-by line to every commit in this branch:

  1. Ensure you have a local copy of your branch by checking out the pull request locally via command line.
  2. In your local branch, run: git rebase HEAD~4 --signoff
  3. Force push your changes to overwrite the branch: git push --force-with-lease origin issue-33/allow-null-input-on-dashboard

Soean and others added 3 commits June 8, 2025 07:58
Signed-off-by: Sören Wünsch <soeren.wrede@parshipgroup.com>
Signed-off-by: Topher DeRosia <topher@mediaforge.pro>
Signed-off-by: Topher DeRosia <topher@mediaforge.pro>
Signed-off-by: Topher DeRosia <topher@mediaforge.pro>
@topher1kenobe topher1kenobe force-pushed the issue-33/allow-null-input-on-dashboard branch from 87e8208 to 9757143 Compare June 8, 2025 11:59
@topher1kenobe
Copy link
Contributor Author

@topher1kenobe The problem is you have two commits that weren’t signed off. I had the same problem when I messed up signing one and the whole PR held on to it.

Did you do this part? The force push was the only way to fix it.

To add your Signed-off-by line to every commit in this branch:

1. Ensure you have a local copy of your branch by [checking out the pull request locally via command line](https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/checking-out-pull-requests-locally).

2. In your local branch, run: `git rebase HEAD~4 --signoff`

3. Force push your changes to overwrite the branch: `git push --force-with-lease origin issue-33/allow-null-input-on-dashboard`

I had tried this once before, but I've done it again. We'll see if it works.

@Ipstenu
Copy link
Contributor

Ipstenu commented Jun 8, 2025

That worked! Looks like it never properly force pushed. No worries, git can get weird.

I’m doing a scan to see how common this error is in plugins. Regardless I think this is a good fix. Should/can we toss in a “doing it wrong” notice for the devs?

@szepeviktor
Copy link
Contributor

Docblock change and null handling is mmissing.

costdev and others added 2 commits June 17, 2025 17:37
Co-authored-by: Joost de Valk <joost@altha.nl>
Signed-off-by: Colin Stewart <79332690+costdev@users.noreply.github.com>
@costdev costdev merged commit d2f2df6 into fairpm:main Jun 17, 2025
45 checks passed
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.

Fatal error on dashboard when plugin passes null instead of string for content
6 participants