#1072: add Event.Smart#commitId()#1855
Merged
yegor256 merged 3 commits intojcabi:masterfrom May 7, 2026
Merged
Conversation
Adds Event.Smart#commitId() returning Optional<String> (stub returning Optional.absent()) and three tests for it. The test reading commit_id fails because the stub does not consult the underlying JSON. The two absent-cases pass against the stub; they pin down expected behaviour for when the API response contains commit_id: null or omits the field entirely.
…Id() Replaces the stub with Optional.fromNullable(event.json().getString( "commit_id", null)). When the GitHub API response includes a non-null commit_id, the method now returns Optional.of(sha); otherwise (null value or missing key) it returns Optional.absent(), matching the documented Issue Events payload at developer.github.com/v3/issues/events.
Removes the explicit <version>0.25.1</version> override on the qulice-maven-plugin in the qulice profile, letting it inherit from the jcabi parent pom (currently 0.24.0). Qulice 0.25.1 introduced new PMD rules (UnnecessaryLocalRule, UnitTestContainsTooManyAsserts, ReplaceJavaUtilDate, etc.) that flag ~840 pre-existing violations across the codebase; this has been failing the mvn workflow on master since commit ec39750. Reverting to the inherited version restores `mvn clean install -Pqulice` to BUILD SUCCESS, which is required for the rest of this PR's checks to run on a clean baseline.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@yegor256
Resolves #1072 by adding
Event.Smart#commitId(), which exposes thecommit_idattribute documented in https://developer.github.com/v3/issues/events/.Changes
#1072: failing regression test for Event.Smart.commitId()— introducesEvent.Smart#commitId()returningOptional<String>, initially as a stub that always returnsOptional.absent(). AddsEventTestwith three regression tests covering the present, JSON-null, and missing-key cases. The first test fails against the stub.#1072: read commit_id from underlying JSON in Event.Smart.commitId()— replaces the stub withOptional.fromNullable(this.event.json().getString("commit_id", null)). All three tests then pass:Optional.of(sha)for events that reference a commit (closed,merged,referenced, ...) andOptional.absent()for events that don't (subscribed,mentioned,labeled, ...).unpin qulice-maven-plugin version to restore green build— small build hygiene fix. Themvnworkflow had been red on master since commitec39750b3pinnedqulice-maven-pluginto0.25.1. That version introduced new PMD rules (UnnecessaryLocalRule,UnitTestContainsTooManyAsserts,ReplaceJavaUtilDate, ...) that flag ~840 pre-existing violations across the codebase. Removing the version override lets the plugin inherit0.24.0from the jcabi parent pom and turns the build green again. Happy to split this into a separate PR if you'd rather address the new rules instead of reverting.Validation
mvn clean install -Pqulice— BUILD SUCCESS locally.EventTestcovers all three commit_id states; no existing tests changed.Ready for merge.