Add BatchStorageInterface::getId()

Added getId(): int to BatchStorageInterface.

The method was already defined on the concrete BatchStorage class and called via the interface type-hint in form.inc and BatchNotFoundTest.php, but was missing from the interface itself.

Please review and let me know if anything more required

I noticed a couple of things that need to be done with this:

• The implementation in BatchStorage should have its docblock changed to @inheritdoc.
• The implementation in BatchStorage throws an \Exception. This needs to be documented in the interface's docblock per our documentation standards.

This is an API change, which needs a change record per our policy.

Thanks for the review, @dcam. I've addressed all three points:

Interface (BatchStorageInterface) — added the getId(): int method declaration, including the @throws \Exception documentation.
Implementation (BatchStorage) — changed the getId() docblock to {@inheritdoc} so it inherits the interface documentation.
Change record — drafted and published: [add CR link here].
No change was needed in the generated proxy class (ProxyClass\Batch\BatchStorage) since it already declares getId(): int, and I confirmed both implementations still satisfy the interface contract.

Pushed to MR !15991. Setting back to Needs review.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Apologies for the noise on this MR, I'm fairly new to contributing and ended up pushing several commits while wrestling with the docblock indentation/alignment to get PHPCS to pass. Thanks for your patience.

The phpcs job is green now, and all of @dcam's review points are addressed:

getId() added to BatchStorageInterface with the @throws \Exception documented
BatchStorage::getId() docblock changed to {@inheritdoc}
Setting to Needs review.

Apologies for the noise on this MR, I'm fairly new to contributing and ended up pushing several commits while wrestling with the docblock indentation/alignment to get PHPCS to pass. Thanks for your patience.

Don't worry about it. Thank you for your contributions and attention to detail.

The code changes look good, but your comment in #10 lacked the link to the change record. Furthermore, the change record would normally be linked automatically in the issue metadata, but it isn't there either. Can you provide a link to it?

Thanks @dcam! I had only drafted the text locally and hadn't published it sorry about that. The change record is now created and linked to this issue: https://www.drupal.org/node/3606576

It's in Draft status right now, please let me know If i need to do anything else, thanks.

You did good work on the change record. Thank you. I made some edits based on experience:

1. I rearranged the text to describe the change that's happened first, rather than initially describing the problem that led to the change. This is something I've seen committers do a lot lately.
2. I removed the code from form.inc and replaced it with a link to the relevant line in the GitLab repo. People read change records in order to find out what action they need to take with their modules or sites. I didn't want anyone to look at that code block and get confused, thinking that it was relevant to their next steps.
3. Similarly, I removed the before and after examples. The ones you added described the changes to the interface. Instead, before/after examples should show others the actions they need to take in order to adapt to the change. This seems like a change that probably needs examples, but to be honest I'm not sure what examples would be relevant. I thought about copying and pasting the getId() code directly from BlockStorage but that contains calls to protected functions not on the interface. So I felt it's inappropriate. Instead I decided to omit examples. We'll see what committers think.

I hope you don't take these points as criticism. I appreciate how thorough you were. A lot of people don't include examples when they should (including me on occasion).

Thanks for taking the time to review and improve it. I really appreciate the feedback the points you mentioned make a lot of sense, especially keeping the change record focused on what maintainers need to do after the change rather than the implementation details.

I’ll keep these guidelines in mind for future change records. Thanks again for the edits and for explaining the reasoning behind them.

Also, if this contribution is eligible, could you please add me as a credited contributor on the issue/commit? I’d appreciate it!

I updated contribution credit.

This is eligible for 11.x backport under the 1-1 rule, so I've gone ahead and committed to main and 11.x, thanks!