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

Refactored the form posting #258

Merged
merged 8 commits into from Nov 30, 2015

Conversation

Projects
None yet
4 participants
@rsandell
Copy link
Member

commented Nov 24, 2015

Replaced most values with DataBoundSetter to make future expansions easier.

Replaced noNameAndEmailParameters and readableMessage with ParameterMode settings so users can choose between Plain, Encoded or No parameter for those parameters instead.

@reviewbybees

String buildStartMessage, String buildSuccessfulMessage, String buildUnstableMessage,
String buildFailureMessage, String buildNotBuiltMessage, String buildUnsuccessfulFilepath, String customUrl,
String serverName, String gerritSlaveId, List<PluginGerritEvent> triggerOnEvents,
boolean dynamicTriggerConfiguration, String triggerConfigURL, String notificationLevel) {

This comment has been minimized.

Copy link
@jglick

jglick Nov 24, 2015

Member

That is the kind of diff hunk I love to see!

@DataBoundConstructor
public GerritTrigger(
List<GerritProject> gerritProjects,
SkipVote skipVote,

This comment has been minimized.

Copy link
@jglick

jglick Nov 24, 2015

Member

Remember to set default values for these fields.

@rsandell

This comment has been minimized.

Copy link
Member Author

commented Nov 26, 2015

The change to enum.jelly is in a pull request to core: jenkinsci/jenkins#1926

rsandell added some commits Nov 24, 2015

Refactored the databound constructor
Replaced most values with DataBoundSetter
to make future expansions easier.
Converted readableMessage to ParameterMode enum
So the user can choose between three options instead of one;
Encode (default), human readable, and none.
Converted noNameAndEmailParameters to ParameterMode enum
So the user can choose between three options instead of one;
Encode (default), human readable, and none.

@rsandell rsandell force-pushed the nosubject branch from 27f7532 to a7d7515 Nov 26, 2015

rsandell added some commits Nov 26, 2015

Moved deprecated fields and readResolve
out of the way a bit and closer together for context

@rsandell rsandell changed the title [WiP] Refactored the form posting Refactored the form posting Nov 27, 2015

@reviewbybees

This comment has been minimized.

Copy link

commented Nov 27, 2015

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

}
//CS IGNORE EmptyBlock FOR NEXT 1 LINES. REASON: Handled one row below
} catch (NullPointerException ignored) { /*Could happen during testing*/ }
if (this.notificationLevel == null) {

This comment has been minimized.

Copy link
@jtnord

jtnord Nov 27, 2015

Member

would it not make more sense to set the default values in the field definition rather than having to put them here?

This comment has been minimized.

Copy link
@rsandell

rsandell Nov 30, 2015

Author Member

They are also set in the field definition, this is just some extra security to get sensible defaults in case the object is created outside of form posting.

@rsandell rsandell force-pushed the nosubject branch from 01be716 to eb92454 Nov 30, 2015

@rsandell rsandell closed this Nov 30, 2015

@rsandell

This comment has been minimized.

Copy link
Member Author

commented Nov 30, 2015

Just tickling the pull request builder

@rsandell rsandell reopened this Nov 30, 2015

rsandell added a commit that referenced this pull request Nov 30, 2015

Merge pull request #258 from jenkinsci/nosubject
Refactored the job-trigger configuration
Replaced most values with DataBoundSetter to make future expansions easier.

Replaced noNameAndEmailParameters and readableMessage with ParameterMode settings so users can choose between Plain, Encoded or No parameter for those parameters instead.

@rsandell rsandell merged commit 26f53f4 into master Nov 30, 2015

1 check passed

Jenkins This pull request looks good
Details

@rsandell rsandell deleted the nosubject branch Nov 30, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.