tuxji commented on pull request #681:
URL: https://github.com/apache/daffodil/pull/681#issuecomment-976606602


   > Even if we autocompile, it still doesn't guarantee that those files are 
committed. Devs still need to `git add` those files. If they use `git add .` or 
something similar that will happen, but it's dev workflow dependent. I 
personally use `git add -p` or manually add files I know I've modified--I don't 
think it would even cross my mind to commit changes I didn't personally make if 
I was new to this. Since it's still possible that a PR is made without actually 
updating these files depending on a dev's workflow, I'd prefer that we still 
have a test somewhere, even if we unconditionally rebuild these when compiling. 
A one line github action could do this.
   
   Yes, dev workflows aren't identical.  I don't leave any modified files lying 
around in my checkout after a push unless I have a very, very good reason for 
deliberately not committing them.  My own workflow is to run `git diff`, write 
the commit message in a file while looking at every difference carefully, and 
then run `git commit -a -F ~/f`.  I use `git add` only if I need to add new 
files not already in git, and that forces me to `git add .`  everything so `git 
diff --staged` will show me all the differences. In practice, the generated 
files will be updates to files already in git and I don't think anyone would 
deliberately hold back from committing the modified example files since they 
must have been making a change to the code generation backend anyway.  If we do 
see contributors leaving out changes to the example files, we can take some 
action (otherwise, YAGNI).
   
   Do note that incrementing the version number could have to commit the 
example files too depending on how sbt constructs the classpath.  If it's 
doable, you could tell sbt to construct the classpath using classes instead of 
jars so the version number doesn't appear in the example files, but I wouldn't 
worry about it.
   
   > However, things get a bit tricky. We can still trigger a task to run 
during compilation without using a generator, but we have to be careful about 
task dependency cycles. The way the genExample needs to work is it essentially 
just forks a java process and executes the CLI with the right arguments. But 
this task depends on the CLI (and all of its dependencies being compiled), so 
this task really can't live in the CLI or any of its dependent subprojects 
(e.g. `daffodil-runtime2` or the root `daffodil` project), or we'll get 
circularity issues. That probably also means that what this task generates 
probably shouldn't be in any of those projects either--even if it works, 
generating a file inside another subproject feels dangerous and might confuse 
sbt, or create subtle issues.
   
   When I was testing object CodeGenerator.main, I had no problem running it 
manually from IDEA with the runtime configuration offering a dropdown allowing 
me to use either daffodil-runtime2's classpath (I got the log4j warning) or 
daffodil-core's classpath (I didn't get the log4j warning but I did get another 
odd warning).  The dropdown offered me the option of using every daffodil 
module's classpath including daffodil-cli, but I know for sure that nothing in 
the runtime2 code generator backend needs anything from daffodil-cli.  Even the 
version number comes from daffodil-runtime2's own jar.  
   
   > Do you mind if I push the changes to your PR once I get things working?
   
   FYI, I wouldn't have minded, but I've just seen your next comment saying 
that you've already added a branch to your fork.  I'll pull it, thanks.  I 
still don't think we need to worry about circular class dependencies, so I 
might try to keep things contained inside daffodil-runtime2.


-- 
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]


Reply via email to