[JEP-234] Customizable header proposal

[imonteroperez]

Highlights

• Proposal on having a customizable header (not only UI capabilities) it is also providing extra functionality and better integration
• See: [JEP-234] Customizable header jenkins#5909

• Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
• Ensure that the pull request title represents the desired changelog entry
• Please describe what you did
• Link to relevant issues in GitHub or Jira
• Link to relevant pull requests, esp. upstream and downstream changes

[oleg-nenashev]

Thanks @imonteroperez ! I am not sure why this particular change requires a JEP. I am not against it. I am requesting changes because of the incorrectly pre-assigned JEP number. With the current numeration strategy it should be rather JEP-234

[imonteroperez]

Thanks @imonteroperez ! I am not sure why this particular change requires a JEP. I am not against it. I am requesting changes because of the incorrectly pre-assigned JEP number. With the current numeration strategy it should be rather JEP-234

Thanks for heads up on numeration! Updated in 781a489

[batmat]

A note on the process: as mentioned in JEP-1 linked from the README, PR normally don't set JEP numbers. This number gets chosen when the PR gets merged.

When they are done, the Draft JEP will have an official JEP number and the submission PR will have been merged to a matching folder (for example, jep/1) in the master branch.

Given Oleg's answer, I assume we can keep 234, but let's remember for next time :)

[batmat]

Overall LGTM. Many wordsmithing suggestions and a few bigger changes proposed.

But again, nothing blocking I could see on the design side of the subject. (hence my approval)

[amuniz]

Looks good.

[jglick]

I assume we can keep 234

Better to use 0000 as directed by JEP-1 and assumed by #376.

[oleg-nenashev]

I will go ahead and merge it

[batmat]

Thank you Oleg!

[batmat]

@oleg-nenashev @jenkinsci/jep-editors can we merge this PR and assign it an official number pretty please? This is ready for merge FWICT, and this is somehow blocking jenkinsci/jenkins#5909 merge (which is ready too).

And it would be cleaner to merge first the JEP before the reference implementation, hence my request here :-).

Thanks!

[oleg-nenashev]

Yes I'm about doing so. Was unable to do it yesterday due to yet another family emergency.

…

On Tue, 23 Nov 2021, 12:23 Baptiste Mathus, ***@***.***> wrote: @batmat <https://github.com/batmat> requested review from @jenkinsci/jep-editors on: #380 <#380> [JEP-234] Customizable header proposal. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#380 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAW4RIAMJNJINP7KZNK7C63UNN2THANCNFSM5H2HI4UA> .

[oleg-nenashev]

Please note that there are at least four JEP editors employed by CloudBees who could do such formalities. The companies will come out to participate in the community governance

…

On Tue, 23 Nov 2021, 12:24 Oleg Nenashev, ***@***.***> wrote: Yes I'm about doing so. Was unable to do it yesterday due to yet another family emergency. On Tue, 23 Nov 2021, 12:23 Baptiste Mathus, ***@***.***> wrote: > @batmat <https://github.com/batmat> requested review from > @jenkinsci/jep-editors on: #380 > <#380> [JEP-234] Customizable > header proposal. > > — > You are receiving this because your review was requested. > Reply to this email directly, view it on GitHub > <#380 (comment)>, or > unsubscribe > <https://github.com/notifications/unsubscribe-auth/AAW4RIAMJNJINP7KZNK7C63UNN2THANCNFSM5H2HI4UA> > . >

[batmat]

@MarkEWaite are you able to do the landing of this PR as a draft JEP? Oleg agrees with the content AFAIK, but would welcome some help on the process if we can.

[MarkEWaite]

I'm happy to do the merge @oleg-nenashev if you're willing to "check my work" that I didn't make some obvious mistake. I believe the merge steps are:

• Create a commit that assigns JEP-234 to this PR (as was done for another JEP in c691756)
• Create a commit that adds JEP-234 to top level README (as was done for another JEP in 42bf67e)
• Merge the branch to master

[timja]

@MarkEWaite
There's a script:
https://github.com/jenkinsci/jep/tree/master/jep/1#approve-as-draft

Probably should be more discoverable than buried in that monster though

[MarkEWaite]

There's a script: https://github.com/jenkinsci/jep/tree/master/jep/1#approve-as-draft

Thanks for that pointer! I adjusted the results slightly (leaving the delegate status as TBD instead of using the value 'not delegated' that was set by the script. If that was a mistake, we can correct it with another pull request.

[MarkEWaite]

This pull request was merged by following the script. After merging it from the command line, the pull request was still open, so I closed it. In the future, I may use the gh pr merge command so that the pull request will be listed by GitHub as "merged" instead of "closed"

[timja]

Looks like its only on your repo atm @MarkEWaite

https://github.com/jenkinsci/jep

[jglick]

The script may need to be adjusted depending on how you name your remotes—I use origin for the @jenkinsci version and fork for my @jglick copy, but there is no standard.

[jglick]

And yes if the script is used correctly, the final git push will automatically result in the PR being considered merged, because GitHub will detect that the PR branch head is now an ancestor of the base branch head.

[MarkEWaite]

I missed the push to upstream. I use an inverted naming convention compared to @jglick. Merging now with the GitHub merge pull request button

[jglick]

Merging now with the GitHub merge pull request button

Also not what the recommended script calls for. It looks like you did some commits by hand to set the number and so on which the proposed script handles a bit differently. Should not matter now, just FYI for next time.