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

Make GitChangeSet serializable (for use in Jenkins Pipeline CPS methods) #1090

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alexdowad
Copy link

Please let me know if there is any reason why this change doesn't make sense! I would have opened a GH 'issue' first for discussion, but it seems that Issues are not enabled on this repository...

Commit message:

"Jenkins requires that objects manipulated in Jenkins Pipeline Groovy methods which are not marked @NoCPS must be serializable. It seems that there is no particular reason why GitChangeSet should not be serializable, so mark it as such."

Checklist

  • I have read the CONTRIBUTING doc
  • I have added tests that verify my changes
  • Unit tests pass locally with my changes
  • No Javadoc warnings were introduced with my changes
  • No spotbugs warnings were introduced with my changes
  • Online help has been added and reviewed for any new or modified fields
  • I have interactively tested my changes

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Jenkins requires that objects manipulated in Jenkins Pipeline Groovy
methods which are not marked @NoCPS must be serializable. It seems that
there is no particular reason why GitChangeSet should not be
serializable, so mark it as such.
@MarkEWaite
Copy link
Contributor

https://issues.jenkins.io/browse/JENKINS-65881 indicates that GitChangeSet cannot be serialized by the Jenkins serialization system because it includes a field that is known to be unsafe for serialization.

@alexdowad
Copy link
Author

@MarkEWaite Thanks so much for the response!! And thanks for the link to the Jenkins bug tracker!

The issue you linked to indicates that the field in GitChangeSet which can't be serialized is dateFormatters, which contains three DateTimeFormatter objects. I'm wondering about that, though... it seems that every single GitChangeSet object contains three equivalent DateTimeFormatters. (They are initialized in the constructor, but the initialization code doesn't use the parameters passed into the constructor at all, so those objects should always be "the same". I wonder if they could even be shared as class fields rather than instance fields. Or else custom serialization methods could be defined which wouldn't serialize those fields out; again, the values are always the same, so there is actually no need to serialize and deserialize them.)

Does that make sense?? I am not familiar with this codebase, so I could easily be totally off base here.

@alexdowad
Copy link
Author

Looks like @MarkEWaite added the code with those DateTimeFormatters in b84c7c9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants