Skip to content

Commit 05b2a5b

Browse files
Jakub RuzickaGerrit Code Review
authored andcommitted
core: refactor unreasonable default atomic=False
In the beginning, I assumed most rdopkg actions would use state and --continue/--abort but that doesn't happen to be the case with only `rdopkg new-version` using this functionality to handle rebases at the moment, so I changed both the default and the name to something less cryptic. continuable=False is the new default for Action() and thus many redundant 'atomic=False' strings were dropped. continuable == not atomic eliminated some negations like if not atomic: in favor of direct if continuable: This is a cleanup of a poor design decision ¯\_(ツ)_/¯ Change-Id: I717f8dedda33d3a43f7326151fd3faf4020f3ade
1 parent e13b207 commit 05b2a5b

File tree

10 files changed

+44
-45
lines changed

10 files changed

+44
-45
lines changed

HACKING.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,6 @@ that's not the case in the time of writing due to legacy reasons.
128128
Finally, note that some refactoring is expected before action modules
129129
interface is stable, not limited to:
130130

131-
* rethink atomic=True which now spams most of actions
132131
* include full action documentation in `__init__.py` as opposed of having
133132
only short string there and the rest in doc/ - that's sure to desync over
134133
time.

doc/rdopkg.1.adoc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,7 @@ ACTION: amend
419419
Amend last git commit with current dist-git changes and (re)generate the commit
420420
message from %changelog.
421421

422-
This simple atomic action is equivalent to running
422+
This simple action is equivalent to running
423423

424424
git commit -a --amend -m "$AUTOMAGIC_COMMIT_MESSAGE"
425425

@@ -433,7 +433,7 @@ ACTION: squash
433433
Squash last git commit into previous one. Commit message of previous commit is
434434
used.
435435

436-
This simple atomic action is a shortcut for
436+
This simple action is a shortcut for
437437

438438
git reset --soft HEAD~
439439
git commit --amend --no-edit

rdopkg/action.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ def cmdarg_names(self, name2argfun):
2121

2222

2323
class Action(object):
24-
def __init__(self, name, steps=None, atomic=False, module=None,
24+
def __init__(self, name, steps=None, continuable=False, module=None,
2525
action_fun=None, required_args=None, optional_args=None,
2626
const_args=None, help=None, description=None):
2727
if not const_args:
@@ -33,7 +33,7 @@ def __init__(self, name, steps=None, atomic=False, module=None,
3333
self.name = name
3434
self.module = module
3535
self.steps = steps
36-
self.atomic = atomic
36+
self.continuable = continuable
3737
self.action_fun = action_fun
3838
self.const_args = const_args
3939
self.required_args = required_args
@@ -178,7 +178,7 @@ def _get_action_fun(self, action):
178178
def run_action(self, action, args=None):
179179
if not args:
180180
args = {}
181-
if not action.atomic:
181+
if action.continuable:
182182
log.info(log.term.bold("## %s" % action.name))
183183
for carg in action.const_args:
184184
args[carg] = action.const_args[carg]

rdopkg/actions/build/__init__.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33

44
ACTIONS = [
5-
Action('coprbuild', atomic=True, help="build package in copr-jruzicka",
5+
Action('coprbuild', help="build package in copr-jruzicka",
66
steps=[
77
Action('get_package_env', module='distgit'),
88
Action('copr_check'),
@@ -18,12 +18,12 @@
1818
Arg('fuser', shortcut='-u',
1919
help="Fedora user to upload srpm as to fedorapeople.org"),
2020
]),
21-
Action('kojibuild', atomic=True, help="build package in koji",
21+
Action('kojibuild', help="build package in koji",
2222
steps=[
2323
Action('get_package_env', module='distgit'),
2424
Action('koji_build'),
2525
]),
26-
Action('cbsbuild', atomic=True, help="build package in CBS",
26+
Action('cbsbuild', help="build package in CBS",
2727
steps=[
2828
Action('get_package_env', module='distgit'),
2929
Action('cbs_build'),
@@ -32,7 +32,7 @@
3232
Arg('scratch', action='store_true',
3333
help='Perform a scratch build'),
3434
]),
35-
Action('mockbuild', atomic=True, help="Run fedpkg/rhpkg mockbuild",
35+
Action('mockbuild', help="Run fedpkg/rhpkg mockbuild",
3636
steps=[
3737
Action('get_package_env', module='distgit'),
3838
Action('fedpkg_mockbuild'),

rdopkg/actions/distgit/__init__.py

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33

44
ACTIONS = [
5-
Action('clone', atomic=True,
5+
Action('clone',
66
help="clone an RDO package distgit and setup remotes",
77
required_args=[
88
Arg('package', positional=True, metavar='PACKAGE',
@@ -17,17 +17,17 @@
1717
Arg('review_user', shortcut='-u', metavar='USER',
1818
help="gerrit username for reviews"),
1919
]),
20-
Action('pkgenv', atomic=True, help="show detected package environment",
20+
Action('pkgenv', help="show detected package environment",
2121
steps=[
2222
Action('get_package_env'),
2323
Action('show_package_env'),
2424
]),
25-
Action('patchlog', atomic=True, help="show patches branch log",
25+
Action('patchlog', help="show patches branch log",
2626
steps=[
2727
Action('get_package_env'),
2828
Action('show_patch_log'),
2929
]),
30-
Action('get-patches', atomic=True,
30+
Action('get-patches',
3131
help="setup local patches branch and switch to it",
3232
optional_args=[
3333
Arg('patches_branch', shortcut='-p', metavar='REMOTE/BRANCH',
@@ -55,7 +55,7 @@
5555
Action('commit_distgit_update'),
5656
Action('final_spec_diff'),
5757
]),
58-
Action('patch', atomic=True,
58+
Action('patch',
5959
help="introduce new patches to the package",
6060
optional_args=[
6161
Arg('patches_branch', shortcut='-p', metavar='REMOTE/BRANCH',
@@ -91,7 +91,8 @@
9191
Action('commit_distgit_update', const_args={'amend': True}),
9292
Action('final_spec_diff'),
9393
]),
94-
Action('new_version', help="update package to new upstream version",
94+
Action('new_version', continuable=True,
95+
help="update package to new upstream version",
9596
optional_args=[
9697
Arg('new_version', positional=True, nargs='?',
9798
help="version to update to"),
@@ -138,7 +139,7 @@
138139
Action('final_spec_diff'),
139140
Action('review_patches_branch')
140141
]),
141-
Action('update_patches', atomic=True,
142+
Action('update_patches',
142143
help='[DEPRECATED] update patches from -patches branch',
143144
description=(
144145
"WARNING: This is a low-level action for backward "
@@ -156,20 +157,20 @@
156157
metavar='LOCAL_BRANCH',
157158
help="local git branch containing patches"),
158159
]),
159-
Action('amend', atomic=True,
160+
Action('amend',
160161
help="amend last commit and recreate commit message",
161162
optional_args=[
162163
Arg('commit_header_file', shortcut='-H', metavar='FILE',
163164
help="start commit message with FILE contents, "
164165
"- for stdin"),
165166
]),
166-
Action('squash', atomic=True,
167+
Action('squash',
167168
help="squash HEAD into HEAD~ using HEAD~ commit message"),
168-
Action('get_source', atomic=True, help="fetch source archive",
169+
Action('get_source', help="fetch source archive",
169170
steps=[
170171
Action('get_source', const_args={'new_sources': True})
171172
]),
172-
Action('tag_patches', atomic=True,
173+
Action('tag_patches',
173174
help='tag the -patches branch in Git with the current NVR',
174175
optional_args=[
175176
Arg('force', shortcut='-f', action='store_true',

rdopkg/actions/info/__init__.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33

44
ACTIONS = [
5-
Action('info', atomic=True, help="show information about RDO packaging",
5+
Action('info', help="show information about RDO packaging",
66
optional_args=[
77
Arg('pkgs', positional=True, nargs='*', metavar='ATTR:REGEX',
88
help="show info about packages with ATTR matching REGEX"),
@@ -13,15 +13,15 @@
1313
Arg('local_info', shortcut='-l',
1414
help="use local rdoinfo repo found in specified path"),
1515
]),
16-
Action('info_tags_diff', atomic=True,
16+
Action('info_tags_diff',
1717
help="find which tags have changed between HEAD~..HEAD in rdoinfo",
1818
optional_args=[
1919
Arg('local_info', positional=True, metavar='RDOINFODIR',
2020
help="use local rdoinfo repo found in RDOINFODIR"),
2121
Arg('buildsys_tags', shortcut='-b', action='store_true',
2222
help="process buildsys-tags instead of regular tags"),
2323
]),
24-
Action('findpkg', atomic=True,
24+
Action('findpkg',
2525
help="find and show single best matching package in rdoinfo",
2626
optional_args=[
2727
Arg('query', positional=True, metavar='PACKAGE/PROJECT/URL',

rdopkg/actions/reqs/__init__.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33

44
ACTIONS = [
5-
Action('reqdiff', atomic=True, help="show diff of requirements.txt",
5+
Action('reqdiff', help="show diff of requirements.txt",
66
steps=[
77
Action('get_package_env', module='distgit'),
88
Action('get_diff_range', module='distgit'),
@@ -15,7 +15,7 @@
1515
"2 args: diff between 1st and 2nd supplied git refs"),
1616
],
1717
),
18-
Action('reqcheck', atomic=True,
18+
Action('reqcheck',
1919
help="inspect requirements.txt vs .spec Requires",
2020
steps=[
2121
Action('get_package_env', module='distgit'),
@@ -25,7 +25,7 @@
2525
Arg('spec', shortcut='-s', action='store_true',
2626
help="output .spec Requires: for easy pasting"),
2727
]),
28-
Action('reqquery', atomic=True,
28+
Action('reqquery',
2929
help="query RDO repos for versions defined in requirements.txt",
3030
required_args=[
3131
Arg('filter', positional=True, metavar='RELEASE(/DIST)',
@@ -52,8 +52,8 @@
5252
Arg('verbose', shortcut='-v', action='store_true',
5353
help="print status during queries"),
5454
]),
55-
Action('query', atomic=True, help="query RDO and distribution repos for "
56-
"available package versions",
55+
Action('query',
56+
help="query RDO/dist repos for available package versions",
5757
optional_args=[
5858
Arg('filter', positional=True, metavar='RELEASE(/DIST)',
5959
help="RDO release(/dist) to query (see `rdopkg info`)"),

rdopkg/actions/review/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
positional=True, nargs='?',
1010
help="local patches branch with changes to review"),
1111
]),
12-
Action('review_spec', atomic=True,
12+
Action('review_spec',
1313
help="send distgit (.spec file) change for review",
1414
optional_args=[
1515
Arg('branch', metavar='DISTGIT_BRANCH',

rdopkg/actions/util/__init__.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@
22

33

44
ACTIONS = [
5-
Action('status', atomic=True, help="show status of previous action"),
6-
Action('conf', atomic=True, help="show rdopkg configuration"),
7-
Action('actions', atomic=True,
5+
Action('status', help="show status of previous action"),
6+
Action('conf', help="show rdopkg configuration"),
7+
Action('actions',
88
help="list action functions and their availability"),
9-
Action('autocomplete', atomic=True,
9+
Action('autocomplete',
1010
help="get TAB completion for rdopkg!"),
11-
Action('doctor', atomic=True,
11+
Action('doctor',
1212
help="activate rdopkg psychoanalyst mode"),
1313
]

rdopkg/core.py

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,7 @@ def __init__(self, action_manager=None, state_file_path=None):
2828
self.state_file_path = state_file_path or const.STATE_FILE_FN
2929

3030
def save_state(self):
31-
if self.action and self.action[0].atomic:
32-
# don't save state for atomic actions
31+
if not (self.action and self.action[0].continuable):
3332
return
3433
data = {
3534
'action': self.action_manager.action_serialize(self.action),
@@ -74,7 +73,7 @@ def clear_state(self, state_file=True, verbose=False):
7473
print("State file removed.")
7574

7675
def _new_action_check(self, new_action):
77-
if self.action and not new_action.atomic:
76+
if self.action and new_action.continuable:
7877
action_name = self.action[0].name
7978
print(log.term.important(
8079
"You're in the middle of previous action: %s\n" %
@@ -105,7 +104,7 @@ def new_action(self, action, args=None):
105104
break
106105
if not self.action:
107106
raise exception.InvalidAction(action=action)
108-
if not action.atomic and action.steps:
107+
if not action.continuable and action.steps:
109108
self.save_state()
110109

111110
def print_progress(self):
@@ -177,10 +176,10 @@ def _check_action(a, indent=''):
177176
def engage(self):
178177
if not self.action:
179178
raise exception.NoActionInProgress
180-
atomic = self.action[0].atomic
179+
continuable = self.action[0].continuable
181180

182181
def _save_state():
183-
if not atomic:
182+
if continuable:
184183
self.save_state()
185184
self.action_manager.ensure_leaf_action(self.action,
186185
on_change_callback=_save_state)
@@ -198,7 +197,7 @@ def _save_state():
198197
select_next = not ex.kwargs.get('rerun', False)
199198
abort = True
200199
except exception.ActionFinished as ex:
201-
self.clear_state(state_file=not atomic)
200+
self.clear_state(state_file=continuable)
202201
print(ex)
203202
return
204203
except exception.ActionGoto as ex:
@@ -212,11 +211,11 @@ def _save_state():
212211
if select_next:
213212
self.action = self.action_manager.next_action(self.action)
214213
if not self.action:
215-
if not atomic:
214+
if continuable:
216215
print("Action finished.")
217-
self.clear_state(state_file=not atomic)
216+
self.clear_state(state_file=continuable)
218217
return
219-
if not atomic:
218+
if continuable:
220219
self.save_state()
221220
if abort:
222221
return

0 commit comments

Comments
 (0)