tuxji commented on PR #919:
URL: https://github.com/apache/daffodil/pull/919#issuecomment-1416425333
I took a quick look. I ran these commands in my terminal:
```shell
interran@GH3WPL13E:~$ cd apache/daffodil-mcgann/
interran@GH3WPL13E:~/apache/daffodil-mcgann$ git pull
Fetching origin
remote: Enumerating objects: 1901, done.
remote: Counting objects: 100% (1860/1860), done.
remote: Compressing objects: 100% (1101/1101), done.
remote: Total 1901 (delta 474), reused 1750 (delta 440), pack-reused 41
Receiving objects: 100% (1901/1901), 2.38 MiB | 3.92 MiB/s, done.
Resolving deltas: 100% (474/474), completed with 166 local objects.
From github.com:mike-mcgann/daffodil
+ 10e5daf65...9937fe68a daffodil-2683-unsplit-packages ->
origin/daffodil-2683-unsplit-packages (forced update)
* [new branch] daffodil-2133-scalafmt ->
origin/daffodil-2133-scalafmt
* [new branch] daffodil-2676.leading_whitespace ->
origin/daffodil-2676.leading_whitespace
2b8e52b82..10c520aef main -> origin/main
fatal: Not possible to fast-forward, aborting.
interran@GH3WPL13E:~/apache/daffodil-mcgann$ git switch
daffodil-2683-unsplit-packages
Already on 'daffodil-2683-unsplit-packages'
Your branch and 'origin/daffodil-2683-unsplit-packages' have diverged,
and have 28 and 25 different commits each, respectively.
(use "git pull" to merge the remote branch into yours)
interran@GH3WPL13E:~/apache/daffodil-mcgann$ git reset --hard
origin/daffodil-2683-unsplit-packages
HEAD is now at 9937fe68a Refactor for OSGI compliance
interran@GH3WPL13E:~/apache/daffodil-mcgann$ git log
commit 9937fe68a44b4ba731e9cdfd58e1c1e93532acc7 (HEAD ->
daffodil-2683-unsplit-packages, origin/daffodil-2683-unsplit-packages)
Author: Mike McGann <[email protected]>
Date: Fri Feb 3 13:10:25 2023 -0500
Refactor for OSGI compliance
Daffodil shares package namespaces across different libraries such that
library A can have a class named foo.bar.One and library B can have a
class named foo.bar.Two. This can cause issues with class loaders that
prohibit different jars from owning the same package (i.e., in an
OSGI container). This factoring ensures that each package is owned by
a single jar.
DAFFODIL-2683
commit 5f778d7084de722a5fcedfae5bfddfa40cdc572f
Author: Mike McGann <[email protected]>
Date: Tue Jan 10 15:37:27 2023 -0500
Add refactor scripts for OSGI compliance
Daffodil shares package namespaces across different libraries such that
library A can have a class named foo.bar.One and library B can have a
class named foo.bar.Two. This can cause issues with class loaders that
prohibit different jars from owning the same package (i.e., in an
OSGI container). These scripts refactor the code to ensure that each
package is only owned by a single jar. An sbt task, osgiCheck, has
been added to enforce compilance in the future.
DAFFODIL-2683
commit d62be6482c8594e8940832005401408b64c169dd
Author: Steve Lawrence <[email protected]>
Date: Thu Feb 2 12:27:52 2023 -0500
Reduce the maximum length of xs:hexBinary to Int.MaxValue / 2
...
interran@GH3WPL13E:~/apache/daffodil-mcgann$ git diff HEAD~2..HEAD~
...
```
The first commit's diff looks good except for one thing. Rat.scala is
changed in both this commit and the next commit. It probably should be changed
only in the second commit, but if no one else objects, it's not a big deal.
```shell
interran@GH3WPL13E:~/apache/daffodil-mcgann$ git diff HEAD~2..HEAD~
project/Rat.scala
...
interran@GH3WPL13E:~/apache/daffodil-mcgann$ git diff HEAD~..HEAD
project/Rat.scala
...
```
You also may need to configure your IDE so that it always appends a newline
as the final character in files. My git diff complained about there being no
newline at the end of the following files:
```
daffodil-runtime1/src/main/scala/org/apache/daffodil/reflection/FieldFinder.scala.md
daffodil-runtime1/src/test/scala/org/apache/daffodil/reflection/TestFieldFinder.scala.md
project/OsgiCheck.scala
scripts/osgi-refactor/.gitignore
scripts/osgi-refactor/fix-imports.scala
```
If Mike asks you to rework something in the first commit, please add
newlines to these files.
The second commit's diffs look like what I saw before, although I noticed
this time that some methods had a local `import NodeInfo._` statement in their
bodies which was turned into an `import
org.apache.daffodil.runtime1.dpath.NodeInfo._` even though there was already an
`import org.apache.daffodil.runtime1.dpath.NodeInfo` at the beginning of these
files. These methods could have kept their original `import NodeInfo._` local
statements without needing to qualify the import. I also saw one `import
InfosetEventKind._` get turned into `import
org.apache.daffodil.runtime1.infoset.InfosetEventKind._`.
Finally, the second commit also changes one line in one of the refactoring
scripts, see `git diff HEAD~..HEAD scripts/osgi-refactor/refactor.sh`. That
change should've been in the first commit. Note that I make it a habit to look
at my own branch's git diff output and write a long commit message describing
why I changed *each* file, which helps me catch unintended changes or rethink
some changes to make them better.
OK, it's Mike's turn now :).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
