Skip to content

Fix polling only when polling is enabled #30

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hbfernandes
Copy link

@hbfernandes hbfernandes commented Nov 13, 2018

From my point of view it does not make sense to have both changelog and poll as false to disable polling. From the tests i've made it seems to work as intended just by changing the condition. Please let me know of any restrictions or problems so that i might try to address them.

@dwnusbaum
Copy link
Member

Thanks for the PR! I agree that the behavior is kind of weird, but I think that this would be a breaking change for various use cases, which we want to avoid. I thought that part of the reason for the current behavior was that for some SCMs, tracking the changelog was the same as enabling polling, so this change wouldn't have the desired behavior in all cases, but I'm not exactly sure.

If you are still interested in this change, can you please add some test cases to show that it works as intended and to show what happens for users who had poll: false and changelog: true previously so we know how it affects existing users?

A simple change that might clear up some of the confusion around the current behavior could be to log a warning when someone uses poll: false with changelog: true explaining that changelog: true overrides poll: false.

@lucendio
Copy link

lucendio commented Aug 5, 2020

@hbfernandes awesome finding! Thanks for sharing.

@dwnusbaum If you are concerted about breaking people's use cases, how would you feel about introducing a new major release, then?

FYI, adjusting both flags in an existing job didn't work for me. I had to create a new job for the polling to not kick in anymore every time a commit landed in the disabled repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants