You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The reason will be displayed to describe this comment to others. Learn more.
Any way we could have a test added which verifies the problem described in the bug report existed before this change? That would help confirm that the fix stays "fixed".
The reason will be displayed to describe this comment to others. Learn more.
This commit is more there to prevent unecessary branch lookup when the build history is sequential. Fixing the reported issue is more a side effect, I indeed would like to understand which scenario can produce the reported issue
The reason will be displayed to describe this comment to others. Learn more.
@KostyaSha thanks to suggest - and do - this
I didn't want to really investigate on this as I'd like BuildData to die, even my attempts on this topic were not successful so far.
The reason will be displayed to describe this comment to others. Learn more.
Done.
I also hate BuildData. But you are committing changes to master without checking them. When you submit PR we can have review and comment code before and also PR provides built binaries that you can provide for testing to initial requester.
The reason will be displayed to describe this comment to others. Learn more.
@KostyaSha I know well the benefits of PR-based workflow, but please remember that, as plugin maintainer, I also have to integrate others PR and do my own stuff ...
The reason will be displayed to describe this comment to others. Learn more.
So what will you do if it wouldn't resolve issue and raise other issues? What is the difference between maintainer and not if you do commits without tests and verification/review?
The reason will be displayed to describe this comment to others. Learn more.
with the amount of reported issue on jira + legacy for git plugin I don't really care about getting new issue reported. Could also reopen.
Maintainer in my vision is not about quality of development workflow, but commitment to focus on a specific plugin on a long time and knowledge about it's history. It's not about being a QA engineer.
The reason will be displayed to describe this comment to others. Learn more.
because git plugin is just 8 years of pull request merged on top of each others and moving maintainers. How can we expect "code quality" in such a context.
We already have considered starting a fresh new git plugin from scratch, but I don't believe in this option.
The reason will be displayed to describe this comment to others. Learn more.
I wrote fresh github-pullrequest plugin (instead ghprb-plugin) and resolving last issues before open-sourcing, but all my issues are because of git-plugin.
The reason will be displayed to describe this comment to others. Learn more.
If you volunteer to write a new git2-plugin that does a better job and can convert existing config for backward compat I'd be happy to deprecate this one.
The reason will be displayed to describe this comment to others. Learn more.
@ndeloof new architecture should do somebody who has big experience to have it modular and extensible. Some ideas like "Build Chooser" are good but probably not enough. Also old plugin -> new plugin migration can be done with some groovy script, but new plugin should cover all features then.
The reason will be displayed to describe this comment to others. Learn more.
if a newer plugin can cover at least some use case an provide automated migration, then that's enough for most of us. The user with exotic usages will just keep the deprecated plugin running.
The reason will be displayed to describe this comment to others. Learn more.
Btw, instead of groovy migration for role-strategy plugin (for converting global-matrix strategy -> role strategy) @oleg-nenashev raised idea about "convert" button in jenkins UI via java code.
For now i can propose only to start covering ideas about how git-plugin should be designed to have good integration with other plugins and use cases.
cfa9180
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any way we could have a test added which verifies the problem described in the bug report existed before this change? That would help confirm that the fix stays "fixed".
cfa9180
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This commit is more there to prevent unecessary branch lookup when the build history is sequential. Fixing the reported issue is more a side effect, I indeed would like to understand which scenario can produce the reported issue
cfa9180
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ndeloof have you tried to ask reporter to provide two build.xml files to look at what is stored in BuildData?
cfa9180
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@KostyaSha thanks to suggest - and do - this
I didn't want to really investigate on this as I'd like BuildData to die, even my attempts on this topic were not successful so far.
cfa9180
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
I also hate BuildData. But you are committing changes to master without checking them. When you submit PR we can have review and comment code before and also PR provides built binaries that you can provide for testing to initial requester.
cfa9180
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@KostyaSha I know well the benefits of PR-based workflow, but please remember that, as plugin maintainer, I also have to integrate others PR and do my own stuff ...
cfa9180
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So what will you do if it wouldn't resolve issue and raise other issues? What is the difference between maintainer and not if you do commits without tests and verification/review?
cfa9180
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with the amount of reported issue on jira + legacy for git plugin I don't really care about getting new issue reported. Could also reopen.
Maintainer in my vision is not about quality of development workflow, but commitment to focus on a specific plugin on a long time and knowledge about it's history. It's not about being a QA engineer.
cfa9180
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not workflow quality, it's code quality.
About history, why BuildData is designed to be copied between every build?
cfa9180
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because git plugin is just 8 years of pull request merged on top of each others and moving maintainers. How can we expect "code quality" in such a context.
We already have considered starting a fresh new git plugin from scratch, but I don't believe in this option.
cfa9180
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wrote fresh github-pullrequest plugin (instead ghprb-plugin) and resolving last issues before open-sourcing, but all my issues are because of git-plugin.
cfa9180
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you volunteer to write a new git2-plugin that does a better job and can convert existing config for backward compat I'd be happy to deprecate this one.
cfa9180
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ndeloof new architecture should do somebody who has big experience to have it modular and extensible. Some ideas like "Build Chooser" are good but probably not enough. Also old plugin -> new plugin migration can be done with some groovy script, but new plugin should cover all features then.
cfa9180
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if a newer plugin can cover at least some use case an provide automated migration, then that's enough for most of us. The user with exotic usages will just keep the deprecated plugin running.
cfa9180
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw, instead of groovy migration for role-strategy plugin (for converting global-matrix strategy -> role strategy) @oleg-nenashev raised idea about "convert" button in jenkins UI via java code.
For now i can propose only to start covering ideas about how git-plugin should be designed to have good integration with other plugins and use cases.
cfa9180
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe bug caused by this change 70857d2 ?