[JIRA] (JENKINS-14568) Improve commit messages from SCM Sync Configuration

Julien Carsique created JENKINS-14568

Improve commit messages from SCM Sync Configuration

Issue Type:	Improvement
Assignee:	Frédéric Camblor
Components:	scm-sync-configuration
Created:	25/Jul/12 12:23 PM
Description:	The generated commit messages are currently useless: << Message: Modification on file by someuser >> I use that plugin to review all changes made on Jenkins, ensuring there was no mistake in a job configuration: I need to quickly see what are the jobs which configuration was changed. Replace "file" with the job name, or at least the effective file name.
Project:	Jenkins
Priority:	Major
Reporter:	Julien Carsique

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators.
For more information on JIRA, see: http://www.atlassian.com/software/jira

Frédéric Camblor commented on JENKINS-14568

Improve commit messages from SCM Sync Configuration

Displaying the job name will be complicated since I am unaware of Saveable implementation current file is related on. I could add the file path in the commit message, but would it be really useful ? Afterall, if you're seeing the commit message, you should be able to see the changeset related to this commit isn't it ? Moreover, it will collide with JENKINS-13613 since I'm going to change scm-sync-configuration commit policy if future release : instead of having 1 file modification change = 1 commit, I'll have several modification changes on several files = 1 commit. So, dusplaying modified file in commit message will be totally useless. WDYT ?

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators.
For more information on JIRA, see: http://www.atlassian.com/software/jira

 

Frédéric Camblor edited a comment on JENKINS-14568

Improve commit messages from SCM Sync Configuration

Displaying the job name will be complicated since I am unaware of Saveable implementation current file is related on. I could add the file path in the commit message, but would it be really useful ? Afterall, if you're seeing the commit message, you should be able to see the changeset related to this commit shouldn't you ? Moreover, it will collide with JENKINS-13613 since I'm going to change scm-sync-configuration commit policy in future release : instead of having 1 file modification change = 1 commit, I'll have several modification changes on several files = 1 commit. So, displaying modified file(s) in commit message will be totally useless. WDYT ?

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators.
For more information on JIRA, see: http://www.atlassian.com/software/jira

Julien Carsique commented on JENKINS-14568

Improve commit messages from SCM Sync Configuration

The file path is enough (and contains the job's name). It would be really useful: I'm working with about 400 jobs (https://qa.nuxeo.org/jenkins/). With so much jobs, developers need the ability to configure jobs themselves, even if I may have to review them latter. I setup an email notification on the Git repository dedicated to SCM Sync Conf (but the readability issue is also true when directly looking at the repository commits history). Because of the current generic message, I cannot quickly review changes: the commits history gives no information and the emails are all threaded together per user (since it's the only information which varies); I have to look at the details for every commit to know which job is involved, that allows unperceived changes on sensible jobs. If I properly understood JENKINS-13613, it will still build a changeset per file/job gathering related Saveable objects instead of generating a commit per object. So it doesn't collide with the current issue: we'll still need to distinguish changesets thanks to their commit message and the related file will be the same for a given changeset (as you wrote: << A Changeset will be made of a Map<filePath, contentOfFile>. Thus, if multiple Saveable work in the same file, they will override same Changeset entry each time the Saveable is saved.>>). What do you mean with << several modification changes on several files = 1 commit >> ? If I misunderstood and you plan to build a unique changeset for multiple changes on multiple jobs, I guess it makes no sense and reduces readability: what about if we need to revert some change on a job or if some people are responsible of reviewing changes on a given group of jobs? Avoiding multiple commits for a single change on a given job is a nice improvement though.

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators.
For more information on JIRA, see: http://www.atlassian.com/software/jira

Frédéric Camblor commented on JENKINS-14568

Improve commit messages from SCM Sync Configuration

You misunderstood the "several modification changes on several files = 1 commit" Once JENKINS-13613 will be implemented, the commit changeset will be made of files modified during the http request of form submission. When you configure a job, only file jobs/foo/config.xml is modified [1], then, only config.xml will be commited. Whereas you're not able to modify several job config files in 1 http request, you won't have more than 1 config.xml file per commit. [1] I simplified the case here to be more clear. In fact, during the jobs form submission, every plugin which plugged themselves on "job-centric" extension point, will have their Saveable.save() method called. Generally, most of them are working into a chunk of the jobs/xxx/config.xml file. Thus, when Saveable.save() is called on 1 plugin, the config.xml is saved on filesystem with new plugin state (and I commit this config.xml state in pre-JENKINS-13613, which explains why we have several commits on same file during a job configuration form submission). Sometimes, the plugins are not working into config.xml file, but in their own xml file. This is the reason why I say we could have multiple files commited during a HTTP request (if some plugin involved are not persisting their data into config.xml file). Note that I always spoke about jobs, but in scm-sync-configuration I never manipulate "jobs" but "saveables" that are a more generic class (a User is a Saveable for instance). That's why we cannot make the jobs workflow a general workflow case. The general workflow case is : Someone submit a form on jenkins Lots of Saveable.save() are called. They modify 1..* files on filesystem. I track these modifications. Once http request is done, scm sync config commit every changed files But everything occurs during 1 http request. When Saveable.save() is not called during a http request (on batches for instance), I won't be able (for the moment) to identify clearly the changesets. Thus, in that particular cases, I'll have to fallback into "pre" JENKINS-13613 behaviour : 1 commit per Saveable.save() In your particular case, wouldn't JENKINS-13613 simplify your life so much that you won't need file path in commit message anymore ?

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators.
For more information on JIRA, see: http://www.atlassian.com/software/jira

Julien Carsique commented on JENKINS-14568

Improve commit messages from SCM Sync Configuration

The planned behavior of JENKINS-13613 sounds perfect. Current issue JENKINS-14568 is still needed: even with JENKINS-13613, I'll have tens of changesets with the same message (or emails within the same thread: one thread per user). Thinking about the job name, the file paths are always of the form jobs/jobName/.../someFile.xml (when configuring jobs) so you could split on slashes ("/") and extract the word after jobs to get the job name. When there is not such a pattern, that means the configuration change wasn't done on a job but on a global config and so the whole file path is nice in that case.

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators.
For more information on JIRA, see: http://www.atlassian.com/software/jira

Frédéric Camblor commented on JENKINS-14568

Improve commit messages from SCM Sync Configuration

I can test "if(currentSaveable instanceof AbstractBuild)" and then cast the Saveable into AbstractBuild when I can retrieve job name (which is more simpler ) But I'm not a big fan of making glitches in code just for this. Isn't your problem due to your user too ? Who doesn't enter any commit message in message prompt ? Eventually, I wouldn't be against a configuration flag allowing to make commit message mandatory if checked. WDYT ?

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators.
For more information on JIRA, see: http://www.atlassian.com/software/jira

 

Frédéric Camblor edited a comment on JENKINS-14568

Improve commit messages from SCM Sync Configuration

I can test "if(currentSaveable instanceof AbstractBuild)" and then cast the Saveable into AbstractBuild where I can retrieve job name (which is more simpler ) But I'm not a big fan of making glitches in code just for this. Isn't your problem due to your user too ? Who doesn't enter any commit message in message prompt ? Eventually, I wouldn't be against a configuration flag allowing to make commit message mandatory if checked. WDYT ?

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators.
For more information on JIRA, see: http://www.atlassian.com/software/jira

Julien Carsique commented on JENKINS-14568

Improve commit messages from SCM Sync Configuration

I guess it's "also" a good idea and I would obviously set that option on. Anyway, I prefer having a reliable information set by the plugin rather than rely on the user's commit message (which is only a bonus, even if mandatory). The instanceof solution seems effectively simpler and is not a glitch at all Note I'm not always asked for a commit message and I didn't check the option to not being bothered anymore by it (I didn't fill an issue for that, nor searched for an existing one).

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators.
For more information on JIRA, see: http://www.atlassian.com/software/jira

Frédéric Camblor commented on JENKINS-14568

Improve commit messages from SCM Sync Configuration

The commit message dialog is only displayed "for the moment" on system configuration page (JENKINS_HOME/configure) and job configuration pages (JENKINS_HOME/jobs/xxx/configure). If you're on one of these pages and your scm-sync-configuration repo credentials are ok, you should have the dialog prompt (otherwise, it is an issue). I dislike the instanceof approach because it is opening the door to unwanted cases like this in the code whereas I used a "generic" extension for Saveables in general (which allow, for instance, to specify user-specific sync'ed paths, see JENKINS-8225). Moreover, I'm pretty sure if I fix current issue, someone else will complain about commit messages which will be too verbose in their particular use of the plugin. To my POV, there isn't any strong reason why I should go either on the left hand or on the right hand, which would benefits in statu quo. I can change my mind if you bring some stronger use cases or votes to this issue.

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators.
For more information on JIRA, see: http://www.atlassian.com/software/jira

 

Frédéric Camblor edited a comment on JENKINS-14568

Improve commit messages from SCM Sync Configuration

You misunderstood the "several modification changes on several files = 1 commit" Once JENKINS-13613 will be implemented, the commit changeset will be made of files modified during the http request of form submission. When you configure a job, only file jobs/foo/config.xml is modified [1], then, only config.xml will be commited. Unless you're not able to modify several job config files in 1 http request, you won't have more than 1 config.xml file per commit. [1] I simplified the case here to be more clear. In fact, during the jobs form submission, every plugin which plugged themselves on "job-centric" extension point, will have their Saveable.save() method called. Generally, most of them are working into a chunk of the jobs/xxx/config.xml file. Thus, when Saveable.save() is called on 1 plugin, the config.xml is saved on filesystem with new plugin state (and I commit this config.xml state in pre-JENKINS-13613, which explains why we have several commits on same file during a job configuration form submission). Sometimes, the plugins are not working into config.xml file, but in their own xml file. This is the reason why I say we could have multiple files commited during a HTTP request (if some plugin involved are not persisting their data into config.xml file). Note that I always spoke about jobs, but in scm-sync-configuration I never manipulate "jobs" but "saveables" that are a more generic class (a User is a Saveable for instance). That's why we cannot make the jobs workflow a general workflow case. The general workflow case is : Someone submit a form on jenkins Lots of Saveable.save() are called. They modify 1..* files on filesystem. I track these modifications. Once http request is done, scm sync config commit every changed files But everything occurs during 1 http request. When Saveable.save() is not called during a http request (on batches for instance), I won't be able (for the moment) to identify clearly the changesets. Thus, in that particular cases, I'll have to fallback into "pre" JENKINS-13613 behaviour : 1 commit per Saveable.save() In your particular case, wouldn't JENKINS-13613 simplify your life so much that you won't need file path in commit message anymore ?

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators.
For more information on JIRA, see: http://www.atlassian.com/software/jira

Julien Carsique commented on JENKINS-14568

Improve commit messages from SCM Sync Configuration

Hi, I really don't see why someone else would complain about too much relevant commit messages... About the strong reason for implementing that feature, please look at the two attached screenshots: they are overviews of a few notifications and the corresponding GitHub history for half a day. Maybe you will see why that is not manageable with reliable information in the commit message. Anyway, I'll fix that in a fork and send a pull-request; do as you want with it. Note I can confirm the absence of the user commit message windows most of the time for all kind of users: JENKINS-14582 Cheers,

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators.
For more information on JIRA, see: http://www.atlassian.com/software/jira

Julien Carsique updated JENKINS-14568

Improve commit messages from SCM Sync Configuration

Screenshots showing the issue of the missing relevant commit message.
Change By:	Julien Carsique (26/Jul/12 12:52 PM)
Attachment:	notifications.tiff
Attachment:	github.tiff

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators.
For more information on JIRA, see: http://www.atlassian.com/software/jira

 

Julien Carsique edited a comment on JENKINS-14568

Improve commit messages from SCM Sync Configuration

Hi, I really don't see why someone else would complain about too much relevant commit messages... About the strong reason for implementing that feature, please look at the two attached screenshots: they are overviews of a few notifications and the corresponding GitHub history for half a day. Maybe you will see why that is not manageable without reliable information in the commit message. Anyway, I'll fix that in a fork and send a pull-request; do as you want with it. Note I can confirm the absence of the user commit message windows most of the time for all kind of users: JENKINS-14582 Cheers,

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators.
For more information on JIRA, see: http://www.atlassian.com/software/jira

Frédéric Camblor commented on JENKINS-14568

Improve commit messages from SCM Sync Configuration

I think your 13 messages on github will be merged into 3 messages once JENKINS-13613 will be implemented. I'm pretty sure it will ease your life. Once implemented, we'll discuss back about this feature.

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators.
For more information on JIRA, see: http://www.atlassian.com/software/jira

Frédéric Camblor resolved JENKINS-14568 as Fixed

Improve commit messages from SCM Sync Configuration

Commit messages should be greatly improved in v0.0.6 I released today. Could you please test it to validate it fits your needs ?
Change By:	Frédéric Camblor (18/Sep/12 12:05 AM)
Status:	Open Resolved
Resolution:	Fixed

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators.
For more information on JIRA, see: http://www.atlassian.com/software/jira

Julien Carsique commented on JENKINS-14568

Improve commit messages from SCM Sync Configuration

Hi, The commit message really looks better but there's a major issue with Maven jobs: all Maven modules' config files are part of the same job configuration and should be committed together one commit per module. just after the upgrade, I received more than two thousand notifications from SCM Sync commits for instance, in the following extract, the job name is "nuxeo-dm-master", not "org.nuxeo.ecm.platform:nuxeo-platform-webapp" which is one of "nuxeo-dm" Maven modules. Message: Job [org.nuxeo.ecm.platform:nuxeo-platform-webapp] configuration updated by SYSTEM Files: A jobs/nuxeo-dm-master/modules/org.nuxeo.ecm.platform$nuxeo-platform-webapp/config.xml diff --git a/jobs/nuxeo-dm-master/modules/org.nuxeo.ecm.platform$nuxeo-platform-webapp/config.xml b/jobs/nuxeo-dm-master/modules/org.nuxeo.ecm.platform$nuxeo-platform-webapp/config.xml new file mode 100644 index 0000000..c831005 --- /dev/null +++ b/jobs/nuxeo-dm-master/modules/org.nuxeo.ecm.platform$nuxeo-platform-webapp/config.xml ... I don't remember if it was already the case, maybe it's only due to a fix (config files not committed by previous versions of the plugin) or a combination of both causes? Since, for now, it seems to add missing files in SCM, I don't know if it will happen again on job configuration changes: if something is changed in the Maven's config files, will there be a new bunch of commits per Maven module? Thanks,

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators.
For more information on JIRA, see: http://www.atlassian.com/software/jira

Frédéric Camblor commented on JENKINS-14568

Improve commit messages from SCM Sync Configuration

Yup this is a limitation of the "transactional commit" feature : It's hard for me to identify a "transaction" when it comes from the SYSTEM (that is, automated modification made by jenkins, in the background). I think it should be better handled during a "human" modification (because in that case, 1 http request = 1 scm transaction). I don't know if you can modify multiple module configurations during 1 http request (I don't have multimodules on my current instance), but if you can, I think there will only be 1 commit.

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators.
For more information on JIRA, see: http://www.atlassian.com/software/jira

Julien Carsique commented on JENKINS-14568

Improve commit messages from SCM Sync Configuration

Confirmed: this only happens on SYSTEM modifications.

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators.
For more information on JIRA, see: http://www.atlassian.com/software/jira

Frédéric Camblor commented on JENKINS-14568

Improve commit messages from SCM Sync Configuration

I should speak with kohsuke and the core team to see if I could find a high level extension point for SYSTEM processes in order to start/end scm transaction on these levels.

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators.
For more information on JIRA, see: http://www.atlassian.com/software/jira