Summary:

Add support for VersionVault

Review Request #11582 — Created April 7, 2021 and submitted Sept. 23, 2021, 2:54 p.m.

Information
Owner:	david
Repository:	RBTools
Branch:	master
Bugs:
Depends On:
Reviewers
Groups:	rbtools
People:

Description

We will soon be adding a new SCMTool implementation to Power Pack to connect to
HCL VersionVault and IBM ClearCase, supplanting the old community-driven
ClearCase support. This change adds RBTools-side support for this.

This is based on code delivered to us in partnership with HCL, but has had
significant changes. The largest change is that instead of having a completely
separate SCMClient implementation in RBTools, this is built into the same
ClearCase client. Most of the code paths are the same between the two types of
backends, and the major difference is that the diff output for VersionVault
includes additional information in the headers (the VOB OID for each changed
element, and a JSON-encoded blob to drop into the FileDiff's extra_data in order
to do better post-processing on the diff display).

There's still more to do for unit testing but functionality should be
relatively complete at this point.

Testing Done


Posted changes from the current checkout (rbt post with no args) including

  new, modified, deleted, renamed, and binary files.
Posted changes that spanned multiple VOBs.
Posted a change that spanned multiple VOBs, including one that was not

  configured as part of the server-side repository. Saw expected

  warning.
Posted changes for UCM activities for both a single and multiple

  revisions.
Ran unit tests.

Commits

Summary
	Add support for VersionVault

Screenshots

Issues

Description	From	Last Updated
E501 line too long (81 > 79 characters)	reviewbot	April 7, 2021, 3:36 p.m.
E722 do not use bare except'	reviewbot	April 7, 2021, 3:36 p.m.
, optional?	chipx86	June 25, 2021, 12:56 p.m.
F401 'rbtools.api.errors.AuthorizationError' imported but unused	reviewbot	April 30, 2021, 3:01 p.m.
Blank line after class docstring.	chipx86	May 11, 2021, 1:47 p.m.
These are so close. Can we build up the cmdline based on self.label and then pass in to execute()?	chipx86	May 11, 2021, 1:49 p.m.
Should be is_ucm.	chipx86	May 11, 2021, 1:49 p.m.
Can you specify explicitly which is which?	chipx86	May 11, 2021, 1:50 p.m.
Can you update this to point to the full class path?	chipx86	May 11, 2021, 1:50 p.m.
E501 line too long (81 > 79 characters)	reviewbot	April 30, 2021, 2:59 p.m.
I think we can just use os.pathsep for this, which is for things like $PATH. Same with the earlier usage …	chipx86	May 11, 2021, 1:53 p.m.
Is Qp part of this?	chipx86	June 30, 2021, 10:04 a.m.
These are pretty close. Can we build up a cmdline and then pass it to execute()?	chipx86	May 11, 2021, 1:55 p.m.
F821 undefined name 're'	reviewbot	April 30, 2021, 3 p.m.
Let's compile the regex right before and then reuse that. I don't think we need the [ and ] though.	chipx86	May 11, 2021, 2:19 p.m.
You can pass a tuple of choices here.	chipx86	May 11, 2021, 2:19 p.m.
You can use os.pathsep here too. Kinda wonder if we should predefine constants for the environment variables on the class …	chipx86	May 11, 2021, 2:19 p.m.
Given how simple this logic is, how about just: if label != 'LATEST' and not self._is_a_label(label, tag): labels_present = False	chipx86	May 11, 2021, 2:20 p.m.
Small nit, but mind alphabetizing these? We also build the string twice. Might as well do it once and then …	chipx86	May 11, 2021, 2:20 p.m.
We can just pass split_lines=True to execute().	chipx86	May 11, 2021, 2:21 p.m.
F821 undefined name 're'	reviewbot	April 30, 2021, 3 p.m.
Just to check, are we splitting on \s, :, or @? Or on \s:@? The [] will give us the …	chipx86	June 30, 2021, 2:38 p.m.
Instead of setting twice, maybe have the second do: extra_data['new'] = extra_data['old'].copy()	chipx86	May 11, 2021, 10:12 a.m.
No need for , None on these.	chipx86	May 11, 2021, 2:22 p.m.
E722 do not use bare except'	reviewbot	April 30, 2021, 3 p.m.
E201 whitespace after '{'	reviewbot	April 30, 2021, 3 p.m.
E202 whitespace before '}'	reviewbot	April 30, 2021, 3 p.m.
E272 multiple spaces before keyword	reviewbot	April 30, 2021, 3 p.m.
E272 multiple spaces before keyword	reviewbot	April 30, 2021, 3 p.m.
F841 local variable 'CLEARCASE_PN' is assigned to but never used	reviewbot	June 25, 2021, 1:35 p.m.
F841 local variable 'commmand' is assigned to but never used	reviewbot	June 25, 2021, 1:20 p.m.
F821 undefined name 'command'	reviewbot	June 25, 2021, 1:23 p.m.
E501 line too long (91 > 79 characters)	reviewbot	June 25, 2021, 1:38 p.m.
E501 line too long (97 > 79 characters)	reviewbot	June 25, 2021, 1:38 p.m.
E501 line too long (97 > 79 characters)	reviewbot	June 25, 2021, 1:38 p.m.
E501 line too long (109 > 79 characters)	reviewbot	June 25, 2021, 1:38 p.m.
E501 line too long (107 > 79 characters)	reviewbot	June 25, 2021, 1:38 p.m.
E501 line too long (115 > 79 characters)	reviewbot	June 25, 2021, 1:38 p.m.
E201 whitespace after '{'	reviewbot	June 25, 2021, 1:19 p.m.
E202 whitespace before '}'	reviewbot	June 25, 2021, 1:19 p.m.
E501 line too long (91 > 79 characters)	reviewbot	June 25, 2021, 1:38 p.m.
E501 line too long (97 > 79 characters)	reviewbot	June 25, 2021, 1:38 p.m.
E501 line too long (97 > 79 characters)	reviewbot	June 25, 2021, 1:38 p.m.
E501 line too long (107 > 79 characters)	reviewbot	June 25, 2021, 1:38 p.m.
E201 whitespace after '{'	reviewbot	June 25, 2021, 1:38 p.m.
E202 whitespace before '}'	reviewbot	June 25, 2021, 1:38 p.m.
F811 redefinition of unused 'test_get_repository_info_dynamic' from line 107	reviewbot	June 25, 2021, 1:19 p.m.
E501 line too long (84 > 79 characters)	reviewbot	June 25, 2021, 1:38 p.m.
E501 line too long (91 > 79 characters)	reviewbot	June 25, 2021, 1:38 p.m.
E501 line too long (97 > 79 characters)	reviewbot	June 25, 2021, 1:38 p.m.
E501 line too long (97 > 79 characters)	reviewbot	June 25, 2021, 1:38 p.m.
E501 line too long (107 > 79 characters)	reviewbot	June 25, 2021, 1:38 p.m.
E201 whitespace after '{'	reviewbot	June 25, 2021, 1:20 p.m.
E202 whitespace before '}'	reviewbot	June 25, 2021, 1:20 p.m.
E501 line too long (84 > 79 characters)	reviewbot	June 25, 2021, 1:38 p.m.
E501 line too long (91 > 79 characters)	reviewbot	June 25, 2021, 1:38 p.m.
E501 line too long (97 > 79 characters)	reviewbot	June 25, 2021, 1:38 p.m.
E501 line too long (97 > 79 characters)	reviewbot	June 25, 2021, 1:38 p.m.
E501 line too long (107 > 79 characters)	reviewbot	June 25, 2021, 1:38 p.m.
E131 continuation line unaligned for hanging indent	reviewbot	June 25, 2021, 1:59 p.m.
E201 whitespace after '{'	reviewbot	June 25, 2021, 1:59 p.m.
E202 whitespace before '}'	reviewbot	June 25, 2021, 1:59 p.m.
Probably should be , optional.	chipx86	July 6, 2021, 2:32 p.m.
The common part was split out, but these are overriding instead of appending to it.	chipx86	July 6, 2021, 2:32 p.m.
No blank line here.	chipx86	July 6, 2021, 2:33 p.m.
Typo: "becaue" -> "because"	chipx86	July 6, 2021, 2:33 p.m.
To ensure doc building works right, the format: should be format::. This will turn this into a code block.	chipx86	July 6, 2021, 2:33 p.m.
I have no idea if it's faster or not, but we can probably just have this be: if '%s/' % …	chipx86	July 6, 2021, 2:33 p.m.
Is "base clearcase" correct? Drawing a blank on the term. Is this just IBM ClearCase?	chipx86	July 6, 2021, 2:34 p.m.
Can we update these to use the formal capitalization of these names?	chipx86	July 6, 2021, 2:35 p.m.
Small nit: Alphabetical order?	chipx86	July 6, 2021, 2:35 p.m.
E303 too many blank lines (2)	reviewbot	July 26, 2021, 1:58 p.m.
E501 line too long (89 > 79 characters)	reviewbot	July 26, 2021, 1:58 p.m.
F401 'json' imported but unused	reviewbot	Aug. 24, 2021, 9:56 p.m.
E501 line too long (80 > 79 characters)	reviewbot	Aug. 24, 2021, 9:55 p.m.
E501 line too long (81 > 79 characters)	reviewbot	Aug. 24, 2021, 9:55 p.m.
E501 line too long (80 > 79 characters)	reviewbot	Aug. 24, 2021, 9:55 p.m.
E501 line too long (81 > 79 characters)	reviewbot	Aug. 24, 2021, 9:55 p.m.
F841 local variable 'diffx_file' is assigned to but never used	reviewbot	Aug. 24, 2021, 9:55 p.m.
F841 local variable 'file_split_re' is assigned to but never used	reviewbot	Aug. 24, 2021, 9:55 p.m.
E501 line too long (81 > 79 characters)	reviewbot	Aug. 24, 2021, 9:55 p.m.
E501 line too long (81 > 79 characters)	reviewbot	Aug. 24, 2021, 9:56 p.m.
Given how old this is, let's nuke it and instead put 3.0, and mention the new support.	chipx86	Sept. 3, 2021, 11:25 a.m.
Definitions generally go at the bottom of the section (the rest of our docs do this).	chipx86	Sept. 17, 2021, 12:12 p.m.
The "3.0" has to be on the next line for section parsing to consider this correctly.	chipx86	Sept. 17, 2021, 12:12 p.m.
Can you add "Version Added"?	chipx86	Sept. 3, 2021, 10 a.m.
I don't know what's really correct, but historically we've put __init__ before @properties. We should probably err on the side …	chipx86	Sept. 3, 2021, 10 a.m.
Minor thing, but should these have quotes around the examples?	chipx86	Sept. 3, 2021, 10 a.m.
Maybe double backticks around teh <...> bits?	chipx86	Sept. 3, 2021, 10:01 a.m.
"a was removed" -> "was removed"	chipx86	Sept. 17, 2021, 12:12 p.m.
This isn't true anymore, right? Given the docs in the function.	chipx86	Sept. 3, 2021, 10:01 a.m.
Is there supposed to be a trailing comma on -predecessor?	chipx86	Sept. 3, 2021, 10:01 a.m.
Can we maek this a standard private function? I feel like this doesn't really need to be inline like this.	chipx86	Sept. 3, 2021, 11:18 a.m.
These could be combined.	chipx86	Sept. 3, 2021, 10:02 a.m.
"Return ..." better describes what this is doing.	chipx86	Sept. 17, 2021, 12:12 p.m.
Given that we never really need i, this can probably just be a standard for f in files.	chipx86	Sept. 3, 2021, 11:22 a.m.
Same as above.	chipx86	Sept. 3, 2021, 11:22 a.m.
Same as above.	chipx86	Sept. 3, 2021, 11:22 a.m.
This is repeated a lot. How about building the base command line outside the loop, then just do cmdline + …	chipx86	Sept. 3, 2021, 11:23 a.m.
Just to check, is the extra space on the 3rd line here intentional? It shows up on the other examples …	chipx86	Sept. 3, 2021, 10:03 a.m.
E265 block comment should start with '# '	reviewbot	Sept. 3, 2021, 11:37 a.m.
F821 undefined name '_get_content_snapshot'	reviewbot	Sept. 3, 2021, 11:37 a.m.
F821 undefined name '_get_content_snapshot'	reviewbot	Sept. 3, 2021, 11:37 a.m.
E303 too many blank lines (2)	reviewbot	Sept. 3, 2021, 11:37 a.m.
F821 undefined name 'i'	reviewbot	Sept. 3, 2021, 11:38 a.m.

flake8 failed.

JSHint passed.

flake8

rbtools/clients/clearcase.py (Diff revision 1)
Show all issues
```
E501 line too long (81 > 79 characters)
```
rbtools/clients/clearcase.py (Diff revision 1)
Show all issues
```
E722 do not use bare except'
```

Summary:

-	WIP integrating ClearCase/VersionVault
+	[WIP] Add support for VersionVault

Description:

~		WIP integrating ClearCase/VersionVault
	~	We will soon be adding a new SCMTool implementation to Power Pack to connect to
	+	HCL VersionVault and IBM ClearCase, supplanting the old community-driven
	+	ClearCase support. This change adds RBTools-side support for this.
	+
	+	This is based on code delivered to us in partnership with HCL, but has had
	+	significant changes. The largest change is that instead of having a completely
	+	separate SCMClient implementation in RBTools, this is built into the same
	+	ClearCase client. Most of the code paths are the same between the two types of
	+	backends, and the major difference is that the diff output for VersionVault
	+	includes additional information in the headers (the VOB OID for each changed
	+	element, and a JSON-encoded blob to drop into the FileDiff's extra_data in order
	+	to do better post-processing on the diff display).
	+
	+	There's still significant testing (manual and automatic) and documentation work
	+	to do on this, hence the WIP.

Commits:

	Summary
-		WIP integrating ClearCase/VersionVault
+		[WIP] Add support for VersionVault

Diff:

Revision 2 (+1452 -452)

Show changes

	rbtools/clients/__init__.py
	rbtools/clients/clearcase.py
	rbtools/clients/tests/test_clearcase.py
	rbtools/commands/diff.py
	rbtools/commands/post.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

rbtools/clients/clearcase.py (Diff revision 2)

Show all issues

F401 'rbtools.api.errors.AuthorizationError' imported but unused

rbtools/clients/clearcase.py (Diff revision 2)
Show all issues
```
E501 line too long (81 > 79 characters)
```
rbtools/clients/clearcase.py (Diff revision 2)
Show all issues
```
F821 undefined name 're'
```
rbtools/clients/clearcase.py (Diff revision 2)
Show all issues
```
F821 undefined name 're'
```
rbtools/clients/clearcase.py (Diff revision 2)
Show all issues
```
E722 do not use bare except'
```
rbtools/clients/clearcase.py (Diff revision 2)
Show all issues
```
E201 whitespace after '{'
```
rbtools/clients/clearcase.py (Diff revision 2)
Show all issues
```
E202 whitespace before '}'
```
rbtools/clients/tests/test_clearcase.py (Diff revision 2)
Show all issues
```
E272 multiple spaces before keyword
```
rbtools/clients/tests/test_clearcase.py (Diff revision 2)
Show all issues
```
E272 multiple spaces before keyword
```

rbtools/clients/__init__.py (Diff revision 2)
Show all issues
```
, optional?
```
rbtools/clients/clearcase.py (Diff revision 2)
Show all issues
```
Blank line after class docstring.
```

rbtools/clients/clearcase.py (Diff revision 2)

Show all issues

These are so close. Can we build up the cmdline based on self.label and then pass in to execute()?

rbtools/clients/clearcase.py (Diff revision 2)
Show all issues
```
Should be is_ucm.
```

rbtools/clients/clearcase.py (Diff revision 2)

Show all issues

Can you specify explicitly which is which?

rbtools/clients/clearcase.py (Diff revision 2)

Show all issues

Can you update this to point to the full class path?

rbtools/clients/clearcase.py (Diff revision 2)

Show all issues

I think we can just use os.pathsep for this, which is for things like $PATH.

Same with the earlier usage as well.

david June 25, 2021, 12:57 p.m.

I suppose it's possible that would work, but I think this handles the common case of cygwin, where os.pathsep is ":" but cleartool will be expecting ";".

rbtools/clients/clearcase.py (Diff revision 2)

Show all issues

Is Qp part of this?

david June 25, 2021, 12:57 p.m.

That's part of the code we got from HCL. I'm not 100% sure one way or the other yet, since my UCM knowledge is still pretty spare.

david June 30, 2021, 10:04 a.m.
```
Answer: Qp is part of it.
```

rbtools/clients/clearcase.py (Diff revision 2)

Show all issues

These are pretty close. Can we build up a cmdline and then pass it to execute()?

rbtools/clients/clearcase.py (Diff revision 2)

Show all issues

Let's compile the regex right before and then reuse that.

I don't think we need the [ and ] though.

rbtools/clients/clearcase.py (Diff revision 2)
Show all issues
```
You can pass a tuple of choices here.
```

rbtools/clients/clearcase.py (Diff revision 2)

Show all issues

You can use os.pathsep here too.
Kinda wonder if we should predefine constants for the environment variables on the class on first use or something, cut down on the conditionals further. Super not important, but just a food-for-thought thing.

rbtools/clients/clearcase.py (Diff revision 2)

Show all issues

Given how simple this logic is, how about just:

if label != 'LATEST' and not self._is_a_label(label, tag):
    labels_present = False

rbtools/clients/clearcase.py (Diff revision 2)

Show all issues

Small nit, but mind alphabetizing these?

We also build the string twice. Might as well do it once and then set.

rbtools/clients/clearcase.py (Diff revision 2)

Show all issues

We can just pass split_lines=True to execute().

rbtools/clients/clearcase.py (Diff revision 2)

Show all issues

Just to check, are we splitting on \s, :, or @? Or on \s:@? The [] will give us the former.

david June 25, 2021, 12:57 p.m.
```
I don't know the answer to this yet.
```

rbtools/clients/clearcase.py (Diff revision 2)

Show all issues

Instead of setting twice, maybe have the second do:

extra_data['new'] = extra_data['old'].copy()

david June 25, 2021, 12:57 p.m.
```
These aren't the same.
```

rbtools/clients/clearcase.py (Diff revision 2)
Show all issues
```
No need for , None on these.
```

Change Summary:

Continued progress

Commits:

	Summary
-		[WIP] Add support for VersionVault
+		Add support for VersionVault

Diff:

Revision 3 (+2204 -472)

Show changes

	rbtools/clients/__init__.py
	rbtools/clients/clearcase.py
	rbtools/clients/tests/test_clearcase.py
	rbtools/commands/diff.py
	rbtools/commands/post.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

```
Warning: Showing 30 of 38 failures.
```

rbtools/clients/clearcase.py (Diff revision 3)

Show all issues

F841 local variable 'CLEARCASE_PN' is assigned to but never used

rbtools/clients/clearcase.py (Diff revision 3)

Show all issues

F841 local variable 'commmand' is assigned to but never used

rbtools/clients/clearcase.py (Diff revision 3)
Show all issues
```
F821 undefined name 'command'
```
rbtools/clients/tests/test_clearcase.py (Diff revision 3)
Show all issues
```
E501 line too long (91 > 79 characters)
```
rbtools/clients/tests/test_clearcase.py (Diff revision 3)
Show all issues
```
E501 line too long (97 > 79 characters)
```
rbtools/clients/tests/test_clearcase.py (Diff revision 3)
Show all issues
```
E501 line too long (97 > 79 characters)
```
rbtools/clients/tests/test_clearcase.py (Diff revision 3)
Show all issues
```
E501 line too long (109 > 79 characters)
```
rbtools/clients/tests/test_clearcase.py (Diff revision 3)
Show all issues
```
E501 line too long (107 > 79 characters)
```
rbtools/clients/tests/test_clearcase.py (Diff revision 3)
Show all issues
```
E501 line too long (115 > 79 characters)
```
rbtools/clients/tests/test_clearcase.py (Diff revision 3)
Show all issues
```
E201 whitespace after '{'
```
rbtools/clients/tests/test_clearcase.py (Diff revision 3)
Show all issues
```
E202 whitespace before '}'
```
rbtools/clients/tests/test_clearcase.py (Diff revision 3)
Show all issues
```
E501 line too long (91 > 79 characters)
```
rbtools/clients/tests/test_clearcase.py (Diff revision 3)
Show all issues
```
E501 line too long (97 > 79 characters)
```
rbtools/clients/tests/test_clearcase.py (Diff revision 3)
Show all issues
```
E501 line too long (97 > 79 characters)
```
rbtools/clients/tests/test_clearcase.py (Diff revision 3)
Show all issues
```
E501 line too long (107 > 79 characters)
```
rbtools/clients/tests/test_clearcase.py (Diff revision 3)
Show all issues
```
E201 whitespace after '{'
```
rbtools/clients/tests/test_clearcase.py (Diff revision 3)
Show all issues
```
E202 whitespace before '}'
```

rbtools/clients/tests/test_clearcase.py (Diff revision 3)

Show all issues

F811 redefinition of unused 'test_get_repository_info_dynamic' from line 107

rbtools/clients/tests/test_clearcase.py (Diff revision 3)
Show all issues
```
E501 line too long (84 > 79 characters)
```
rbtools/clients/tests/test_clearcase.py (Diff revision 3)
Show all issues
```
E501 line too long (91 > 79 characters)
```
rbtools/clients/tests/test_clearcase.py (Diff revision 3)
Show all issues
```
E501 line too long (97 > 79 characters)
```
rbtools/clients/tests/test_clearcase.py (Diff revision 3)
Show all issues
```
E501 line too long (97 > 79 characters)
```
rbtools/clients/tests/test_clearcase.py (Diff revision 3)
Show all issues
```
E501 line too long (107 > 79 characters)
```
rbtools/clients/tests/test_clearcase.py (Diff revision 3)
Show all issues
```
E201 whitespace after '{'
```
rbtools/clients/tests/test_clearcase.py (Diff revision 3)
Show all issues
```
E202 whitespace before '}'
```
rbtools/clients/tests/test_clearcase.py (Diff revision 3)
Show all issues
```
E501 line too long (84 > 79 characters)
```
rbtools/clients/tests/test_clearcase.py (Diff revision 3)
Show all issues
```
E501 line too long (91 > 79 characters)
```
rbtools/clients/tests/test_clearcase.py (Diff revision 3)
Show all issues
```
E501 line too long (97 > 79 characters)
```
rbtools/clients/tests/test_clearcase.py (Diff revision 3)
Show all issues
```
E501 line too long (97 > 79 characters)
```
rbtools/clients/tests/test_clearcase.py (Diff revision 3)
Show all issues
```
E501 line too long (107 > 79 characters)
```

Change Summary:

Review Bot

Commits:

	Summary
-		Add support for VersionVault
+		Add support for VersionVault

Diff:

Revision 4 (+2300 -472)

Show changes

	rbtools/clients/__init__.py
	rbtools/clients/clearcase.py
	rbtools/clients/tests/test_clearcase.py
	rbtools/commands/diff.py
	rbtools/commands/post.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

rbtools/clients/tests/test_clearcase.py (Diff revision 4)
Show all issues
```
E131 continuation line unaligned for hanging indent
```
rbtools/clients/tests/test_clearcase.py (Diff revision 4)
Show all issues
```
E201 whitespace after '{'
```
rbtools/clients/tests/test_clearcase.py (Diff revision 4)
Show all issues
```
E202 whitespace before '}'
```

Change Summary:

More Review Bot

Commits:

	Summary
-		Add support for VersionVault
+		Add support for VersionVault

Diff:

Revision 5 (+2300 -472)

Show changes

	rbtools/clients/__init__.py
	rbtools/clients/clearcase.py
	rbtools/clients/tests/test_clearcase.py
	rbtools/commands/diff.py
	rbtools/commands/post.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:


Fix up revision handling and parsing for UCM activities.
Add multi-vob support for activities.
Add new handling for checked-out files inside the current activity.
Fix directory element skipping when using dynamic views.

Commits:

	Summary
-		Add support for VersionVault
+		Add support for VersionVault

Diff:

Revision 6 (+2390 -554)

Show changes

	rbtools/clients/__init__.py
	rbtools/clients/clearcase.py
	rbtools/clients/tests/test_clearcase.py
	rbtools/commands/diff.py
	rbtools/commands/post.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:


Simplify branch name parsing when posting a stream.
Skip adding upmodified files to the diff (but print a warning).

Commits:

	Summary
-		Add support for VersionVault
+		Add support for VersionVault

Diff:

Revision 7 (+2398 -556)

Show changes

	rbtools/clients/__init__.py
	rbtools/clients/clearcase.py
	rbtools/clients/tests/test_clearcase.py
	rbtools/commands/diff.py
	rbtools/commands/post.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

This is looking great. A few small things.

rbtools/clients/__init__.py (Diff revision 7)
Show all issues
```
Probably should be , optional.
```

rbtools/clients/clearcase.py (Diff revision 7)

Show all issues

The common part was split out, but these are overriding instead of appending to it.

rbtools/clients/clearcase.py (Diff revision 7)
Show all issues
```
No blank line here.
```
rbtools/clients/clearcase.py (Diff revision 7)
Show all issues
```
Typo: "becaue" -> "because"
```

rbtools/clients/clearcase.py (Diff revision 7)

Show all issues

To ensure doc building works right, the format: should be format::. This will turn this into a code block.

rbtools/clients/clearcase.py (Diff revision 7)

Show all issues

I have no idea if it's faster or not, but we can probably just have this be:

if '%s/' % tag in path:
    break

rbtools/clients/clearcase.py (Diff revision 7)

Purely a suggestion, and not important for this change: Might be nice to have a _run_cleartool() command or something that takes the vobs or other things we need and builds them in a platform-dependent way in one place, so we can consolidate this sort of logic.
The environment variable constants we define elsewhere could maybe be done on a class level.

rbtools/clients/tests/test_clearcase.py (Diff revision 7)

Show all issues

Is "base clearcase" correct? Drawing a blank on the term. Is this just IBM ClearCase?

david July 6, 2021, 2:53 p.m.

Yeah, it's correct. The distinction here is base clearcase vs. UCM

rbtools/clients/tests/test_clearcase.py (Diff revision 7)

Show all issues

Can we update these to use the formal capitalization of these names?

rbtools/commands/diff.py (Diff revision 7)
Show all issues
```
Small nit: Alphabetical order?
```

Change Summary:


Add in documentation for various posting modes.
Fix for when /main/1 has been rmvered.
Fix for finding the correct predecessor version in a UCM activity when revisions have been rmvered.
Fix up various comments and other minor things.

Summary:

-	[WIP] Add support for VersionVault
+	Add support for VersionVault

Description:

		We will soon be adding a new SCMTool implementation to Power Pack to connect to
		HCL VersionVault and IBM ClearCase, supplanting the old community-driven
		ClearCase support. This change adds RBTools-side support for this.

		This is based on code delivered to us in partnership with HCL, but has had
		significant changes. The largest change is that instead of having a completely
		separate SCMClient implementation in RBTools, this is built into the same
		ClearCase client. Most of the code paths are the same between the two types of
		backends, and the major difference is that the diff output for VersionVault
		includes additional information in the headers (the VOB OID for each changed
		element, and a JSON-encoded blob to drop into the FileDiff's extra_data in order
		to do better post-processing on the diff display).
-
-		There's still significant testing (manual and automatic) and documentation work
-		to do on this, hence the WIP.

Testing Done:

	+	Posted changes from the current checkout (rbt post with no args) including new, modified, deleted, and binary files.
	+	Posted changes that spanned multiple VOBs.
	+	Posted a change that spanned multiple VOBs, including one that was not configured as part of the server-side repository. Saw expected warning.
	+	Posted changes for UCM activities for both a single and multiple revisions.
	+
	+	More testing to come.

Commits:

	Summary
-		Add support for VersionVault
+		Add support for VersionVault

Diff:

Revision 8 (+2776 -600)

Show changes

	docs/rbtools/rbt/commands/post.rst
	rbtools/clients/__init__.py
	rbtools/clients/clearcase.py
	rbtools/clients/tests/test_clearcase.py
	rbtools/commands/diff.py
	rbtools/commands/post.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

rbtools/clients/tests/test_clearcase.py (Diff revision 8)
Show all issues
```
E303 too many blank lines (2)
```
rbtools/clients/tests/test_clearcase.py (Diff revision 8)
Show all issues
```
E501 line too long (89 > 79 characters)
```

Commits:

	Summary
-		Add support for VersionVault
+		Add support for VersionVault

Diff:

Revision 9 (+2776 -600)

Show changes

	docs/rbtools/rbt/commands/post.rst
	rbtools/clients/__init__.py
	rbtools/clients/clearcase.py
	rbtools/clients/tests/test_clearcase.py
	rbtools/commands/diff.py
	rbtools/commands/post.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:


Switch to DiffX
Add support for renamed and deleted files
Lots of bugfixing and cleanup

Description:

		We will soon be adding a new SCMTool implementation to Power Pack to connect to
		HCL VersionVault and IBM ClearCase, supplanting the old community-driven
		ClearCase support. This change adds RBTools-side support for this.

		This is based on code delivered to us in partnership with HCL, but has had
		significant changes. The largest change is that instead of having a completely
		separate SCMClient implementation in RBTools, this is built into the same
		ClearCase client. Most of the code paths are the same between the two types of
		backends, and the major difference is that the diff output for VersionVault
		includes additional information in the headers (the VOB OID for each changed
		element, and a JSON-encoded blob to drop into the FileDiff's extra_data in order
		to do better post-processing on the diff display).
	+
	+	There's still more to do for unit testing but functionality should be
	+	relatively complete at this point.

Testing Done:

~		Posted changes from the current checkout (rbt post with no args) including new, modified, deleted, and binary files.
	~	Posted changes from the current checkout (rbt post with no args) including new, modified, deleted, renamed, and binary files.
		Posted changes that spanned multiple VOBs.
		Posted a change that spanned multiple VOBs, including one that was not configured as part of the server-side repository. Saw expected warning.
		Posted changes for UCM activities for both a single and multiple revisions.
~
	~	Ran unit tests.
-		More testing to come.

Commits:

	Summary
-		Add support for VersionVault
+		Add support for VersionVault

Diff:

Revision 10 (+3694 -712)

Show changes

	dev-requirements.txt
	setup.py
	docs/rbtools/rbt/commands/post.rst
	rbtools/clients/__init__.py
	rbtools/clients/clearcase.py
	rbtools/clients/tests/test_clearcase.py
	rbtools/commands/diff.py
	rbtools/commands/post.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

rbtools/clients/clearcase.py (Diff revision 10)
Show all issues
```
F401 'json' imported but unused
```
rbtools/clients/clearcase.py (Diff revision 10)
Show all issues
```
E501 line too long (80 > 79 characters)
```
rbtools/clients/clearcase.py (Diff revision 10)
Show all issues
```
E501 line too long (81 > 79 characters)
```
rbtools/clients/clearcase.py (Diff revision 10)
Show all issues
```
E501 line too long (80 > 79 characters)
```
rbtools/clients/clearcase.py (Diff revision 10)
Show all issues
```
E501 line too long (81 > 79 characters)
```

rbtools/clients/clearcase.py (Diff revision 10)

Show all issues

F841 local variable 'diffx_file' is assigned to but never used

rbtools/clients/clearcase.py (Diff revision 10)

Show all issues

F841 local variable 'file_split_re' is assigned to but never used

rbtools/clients/clearcase.py (Diff revision 10)
Show all issues
```
E501 line too long (81 > 79 characters)
```
rbtools/clients/clearcase.py (Diff revision 10)
Show all issues
```
E501 line too long (81 > 79 characters)
```

Change Summary:

ReviewBot

Commits:

	Summary
-		Add support for VersionVault
+		Add support for VersionVault

Diff:

Revision 11 (+3698 -712)

Show changes

	dev-requirements.txt
	setup.py
	docs/rbtools/rbt/commands/post.rst
	rbtools/clients/__init__.py
	rbtools/clients/clearcase.py
	rbtools/clients/tests/test_clearcase.py
	rbtools/commands/diff.py
	rbtools/commands/post.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

docs/rbtools/rbt/commands/post.rst (Diff revision 11)

Show all issues

Given how old this is, let's nuke it and instead put 3.0, and mention the new support.

rbtools/clients/clearcase.py (Diff revision 11)
Show all issues
```
Can you add "Version Added"?
```

rbtools/clients/clearcase.py (Diff revision 11)

Show all issues

I don't know what's really correct, but historically we've put __init__ before @properties. We should probably err on the side of consistency.

rbtools/clients/clearcase.py (Diff revision 11)

Show all issues

Minor thing, but should these have quotes around the examples?

rbtools/clients/clearcase.py (Diff revision 11)

Show all issues

Maybe double backticks around teh <...> bits?

rbtools/clients/clearcase.py (Diff revision 11)

Show all issues

This isn't true anymore, right? Given the docs in the function.

rbtools/clients/clearcase.py (Diff revision 11)

Show all issues

Is there supposed to be a trailing comma on -predecessor?

rbtools/clients/clearcase.py (Diff revision 11)

Show all issues

Can we maek this a standard private function? I feel like this doesn't really need to be inline like this.

rbtools/clients/clearcase.py (Diff revision 11)
Show all issues
```
These could be combined.
```

rbtools/clients/clearcase.py (Diff revision 11)

Show all issues

Given that we never really need i, this can probably just be a standard for f in files.

rbtools/clients/clearcase.py (Diff revision 11)
Show all issues
```
Same as above.
```
rbtools/clients/clearcase.py (Diff revision 11)
Show all issues
```
Same as above.
```

rbtools/clients/clearcase.py (Diff revision 11)

Show all issues

This is repeated a lot. How about building the base command line outside the loop, then just do cmdline + [old_file] (etc)?

rbtools/clients/tests/test_clearcase.py (Diff revision 11)

Show all issues

Just to check, is the extra space on the 3rd line here intentional? It shows up on the other examples below as well.

david Sept. 3, 2021, 11:25 a.m.

This is all copied out of actual cleartool output, so I'd rather keep it even though it's weird.

Commits:

	Summary
-		Add support for VersionVault
+		Add support for VersionVault

Diff:

Revision 12 (+3740 -720)

Show changes

	dev-requirements.txt
	setup.py
	docs/rbtools/rbt/commands/post.rst
	rbtools/clients/__init__.py
	rbtools/clients/clearcase.py
	rbtools/clients/tests/test_clearcase.py
	rbtools/commands/diff.py
	rbtools/commands/post.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

rbtools/clients/clearcase.py (Diff revision 12)
Show all issues
```
E265 block comment should start with '# '
```
rbtools/clients/clearcase.py (Diff revision 12)
Show all issues
```
F821 undefined name '_get_content_snapshot'
```
rbtools/clients/clearcase.py (Diff revision 12)
Show all issues
```
F821 undefined name '_get_content_snapshot'
```
rbtools/clients/clearcase.py (Diff revision 12)
Show all issues
```
E303 too many blank lines (2)
```
rbtools/clients/clearcase.py (Diff revision 12)
Show all issues
```
F821 undefined name 'i'
```

Commits:

	Summary
-		Add support for VersionVault
+		Add support for VersionVault

Diff:

Revision 13 (+3736 -720)

Show changes

	dev-requirements.txt
	setup.py
	docs/rbtools/rbt/commands/post.rst
	rbtools/clients/__init__.py
	rbtools/clients/clearcase.py
	rbtools/clients/tests/test_clearcase.py
	rbtools/commands/diff.py
	rbtools/commands/post.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

docs/rbtools/rbt/commands/post.rst (Diff revisions 11 - 13)

Show all issues

Definitions generally go at the bottom of the section (the rest of our docs do this).

rbtools/clients/clearcase.py (Diff revisions 11 - 13)

Show all issues

The "3.0" has to be on the next line for section parsing to consider this correctly.

rbtools/clients/clearcase.py (Diff revisions 11 - 13)
Show all issues
```
"a was removed" -> "was removed"
```
rbtools/clients/clearcase.py (Diff revisions 11 - 13)
Show all issues
```
"Return ..." better describes what this is doing.
```

Commits:

	Summary
-		Add support for VersionVault
+		Add support for VersionVault

Diff:

Revision 14 (+3738 -720)

Show changes

	dev-requirements.txt
	setup.py
	docs/rbtools/rbt/commands/post.rst
	rbtools/clients/__init__.py
	rbtools/clients/clearcase.py
	rbtools/clients/tests/test_clearcase.py
	rbtools/commands/diff.py
	rbtools/commands/post.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to master (5f733db)