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

Provide ignore/unignore request staging commands. #631

Merged
merged 1 commit into from Jan 12, 2017

Conversation

@jberry-suse
Copy link
Contributor

jberry-suse commented Jan 10, 2017

It is not uncommon for a request to be in a pending state which requires action beyond the scope of the staging workflow, but does not make sense to deny the request. For lack of a "postponed" or "pending" state on OBS a sudo-state of "ignore" is provided for the staging workflow. The ignore state will remove a request from the "list" and "adi" commands so as not to be accidentally staged. This avoids the need for keeping a context of what requests should be ignored in one's memory.

It is expected that an ignored request will have comments reflecting what is to be done or one should be added via the -m option of ignore command.

During our weekly call @lnussel seemed to agree that such a facility would be useful.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jan 10, 2017

Coverage Status

Coverage decreased (-0.2%) to 44.119% when pulling 2ea6f39 on jberry-suse:request-ignore into 55ae250 on openSUSE:master.

@jberry-suse

This comment has been minimized.

Copy link
Contributor Author

jberry-suse commented Jan 10, 2017

I imagine removing from project/staging_projects/$project backlog list would be possible and useful as well.

I had implemented a wrapper around osc staging that included such an ignore list and have found it rather handy.

@jberry-suse

This comment has been minimized.

Copy link
Contributor Author

jberry-suse commented Jan 10, 2017

For testing I went ahead and did so for SR 447422 in openSUSE:Leap:42.3.

@lnussel

This comment has been minimized.

Copy link
Member

lnussel commented Jan 10, 2017

IMO good idea. Looks like an ignored request vanishes from the list completely and forever. How can we make sure we don't forget about it and re-evaluate after some time if the maintainer doesn't respond?
Maybe we should call it suspend and put some timeout?

@nilxam

This comment has been minimized.

Copy link
Contributor

nilxam commented Jan 10, 2017

I'd prefer don't ignore it in the list command...just add an additional mark or text for the request is ignored in the list command.

@DimStar77

This comment has been minimized.

Copy link
Contributor

DimStar77 commented Jan 10, 2017

I agree with @nilxam : hiding them away in the osc staging list overview means we have to think about 'getting them back into the game'; it would be more helpful to have a 'reason why it's still, or again, on the backlog' (stuff we currently put in the comments on the SR, but often enough not checked when doing 'staging list'

@aplanas

This comment has been minimized.

Copy link
Contributor

aplanas commented Jan 10, 2017

I think that is missing a command to list the content of the ignored list, and a easy way to wipe out the content

@nilxam

This comment has been minimized.

Copy link
Contributor

nilxam commented Jan 10, 2017

IMO integrate ignored list into list command should good enough to understand, and select an ignored request by select command will auto-remove the request from the ignored list is better.

@jberry-suse

This comment has been minimized.

Copy link
Contributor Author

jberry-suse commented Jan 11, 2017

Presenting the list somewhere definitely makes sense as I considered another command to list them. Annotating the list command output seems like a good plan. On the adi front a similar annotation while continuing to ignore for automatic selection into an adi staging. Automatically removing from ignore list when selected seems like a good addition. I also thought about checking the existing request in list and cleaning them up when another is ignored. For example if a request ended up being denied in the mean time.

As for placing the reason in the ignore list, I see the usefulness and considered, but I figured we are duplicating the information that should be present in comments so others not using staging tools can see it. Could require the message parameter and place a comment on the request and in the ignore file for this purpose, or try and read an annotated comment from request. Thoughts?

It is not uncommon for a request to be in a pending state which requires
action beyond the scope of the staging workflow, but does not make sense
to deny the request. For lack of a "postponed" or "pending" state on OBS
a sudo-state of "ignore" is provided for the staging workflow. The ignore
state will remove a request from the "list" and "adi" commands so as not
to be accidentally staged. This avoids the need for keeping a context of
what requests should be ignored in one's memory.

It is expected that an ignored request will have comments reflecting what
is to be done or one should be added via the -m option of ignore command.
@jberry-suse jberry-suse force-pushed the jberry-suse:request-ignore branch from 2ea6f39 to b25fda9 Jan 12, 2017
@coveralls

This comment has been minimized.

Copy link

coveralls commented Jan 12, 2017

Coverage Status

Coverage decreased (-0.5%) to 46.307% when pulling b25fda9 on jberry-suse:request-ignore into d21dbfb on openSUSE:master.

@jberry-suse

This comment has been minimized.

Copy link
Contributor Author

jberry-suse commented Jan 12, 2017

I tried to take into account the suggestions and reworks it a bit. The --message option is now stored in ignored_requests file so it can easily be displayed when using list or adi commands. When an ignored request is selected it should automatically be un-ignored (I need to confirm this works).

Some example output:

$ osc staging -p openSUSE:Leap:42.3 list
YaST:Head
  sr#449500: yast2-ntp-client               -> 1-MinimalX   (Factory)
    ignored: depends on augeas-lenses update, see previously sr#447422
network
  sr#449747: bind                           -> 1-MinimalX   (SP1:Update)

I am torn on what should be done with Not in a ring: output on list command. Either duplicate what is in adi command, hide from list there, or leave as is and just display them with no annotation there since primary focus is ring packages.

@jberry-suse

This comment has been minimized.

Copy link
Contributor Author

jberry-suse commented Jan 12, 2017

See the referenced ignored_requests file for an example of the simple yaml storage format.

@jberry-suse

This comment has been minimized.

Copy link
Contributor Author

jberry-suse commented Jan 12, 2017

Could rework as @property like ring_packages if desired.

@lnussel lnussel merged commit 47a20b8 into openSUSE:master Jan 12, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jberry-suse jberry-suse deleted the jberry-suse:request-ignore branch Feb 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.