One thing I'm hoping there is a way to do. I'd like to see all the files in a 
review in a tree view.


Using fisheye on NCSA the review window has a tree-view of the complete change 
set on the left, so you can jump around the change in a logical order based on 
where and what all the changes are. E.g., you can start from the tests, or jump 
into the runtime parts, first, then circle back to other parts, etc.


I am hoping there's a way to achieve this using the github infrastructure. 
Daffodil still gets many changes that involve 60+ files, so that tree view is 
very important. The basic github view of a patch set seems to be oriented 
toward small changes that touch only a small number of files.


The alternative I know of is to pull the branch into one's local repo. Then you 
can view the change using an IDE. That misses out on the ability to rapidly 
comment on it, and muddies the waters of what has changed, vs. what has stayed 
the same; however, it does also help very much with navigating a large patch 
set as one uses the same tools one uses when programming/debugging.


Is there a third (fourth, fifth...) way to do this that would be more like the 
fisheye experience, i.e., a middle ground between just file-by file patches, 
and pulling the branch and using ordinary IDE?



________________________________
From: mbeckerle <[email protected]>
Sent: Tuesday, October 31, 2017 11:23:41 AM
To: [email protected]
Subject: [GitHub] incubator-daffodil pull request #1: Property Resolution - 
part of fixing sch...

Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/1#discussion_r148030522

    --- Diff: 
daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dpath/Expression.scala 
---
    @@ -294,8 +294,8 @@ case class ComparisonExpression(op: String, adds: 
List[Expression])
           case ("=", _) => subsetError("Unsupported operation '%s'. Use 'eq' 
instead.", op)
           case ("!=", _) => subsetError("Unsupported operation '%s'. Use 'ne' 
instead.", op)

    -      case("eq", HexBinary) => EQ_CompareByteArray
    -      case("ne", HexBinary) => NE_CompareByteArray
    +      case ("eq", HexBinary) => EQ_CompareByteArray
    +      case ("ne", HexBinary) => NE_CompareByteArray
    --- End diff --

    Thank you for that useful comment. (Just a test of comments and replies 
while I figure out this code-review system.)


---

Reply via email to