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

automation: Initialise default values #5811

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ricekot
Copy link
Member

@ricekot ricekot commented Oct 11, 2024

No description provided.

Comment on lines -38 to -40
Object value = writer.getMember().getValue(pojo);
if (value == null
|| (value instanceof Collection && ((Collection<?>) value).isEmpty())
Copy link
Member Author

@ricekot ricekot Oct 11, 2024

Choose a reason for hiding this comment

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

We wouldn't need these checks if defaults are set correctly for all jobs. Also, for fields where the default is a non-null value or a non-empty list, these checks prevent the user from overriding those fields with a null/empty value.

public static class Data extends JobData {
private Parameters parameters;
private final Parameters parameters;
Copy link
Member Author

Choose a reason for hiding this comment

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

Wdyt about moving parameters to JobData? That will ensure that we are consistent with the field name across jobs. Each job already provides its parameters (by overriding getParameters).

Copy link
Member

@psiinon psiinon Oct 11, 2024

Choose a reason for hiding this comment

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

We have to support backwards compatibility.
If thats maintained then we should go with whatever structure we think makes the most sense 😁
Oh, and without changing things to often 😉

Copy link
Member

@thc202 thc202 Oct 11, 2024

Choose a reason for hiding this comment

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

How is it going to be declared in the JobData? It's not the same type in all jobs, though they are all AutomationData.

Copy link
Member Author

@ricekot ricekot Oct 14, 2024

Choose a reason for hiding this comment

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

Right, I was thinking of using AutomationData...

But that can probably be in a separate PR. For this one I'll focus on the default values.

Signed-off-by: ricekot <git@ricekot.com>
@ricekot ricekot changed the title automation: Initialise default values [WIP] automation: Initialise default values Oct 16, 2024
Signed-off-by: ricekot <github@ricekot.com>
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