Conversation
topherbuckley
left a comment
topherbuckley
left a comment
There was a problem hiding this comment.
I'm fine with everything other than the changes to the download_models.gradle file which will affect more than just your example app.
As there is no rush to merge to the main branch, I'd also prefer if you gave me some time to review before merging. As I'm not actively working on this now, I typically only check once a week or so. You can still use all your changes from your own branch until then.
| tasks.register('downloadModelFiles') { | ||
| doLast { | ||
| MODEL_FILES.each { file -> | ||
| download.run { | ||
| src file.url | ||
| dest "${ASSET_DIR}/${file.name}" | ||
| overwrite false | ||
| } |
There was a problem hiding this comment.
I'd prefer if you keep these kinds of fundamental changes (change that affect more than just this example app) in a separate PR. Otherwise this will be much harder to track down if this causes issues later on.
There was a problem hiding this comment.
you're right at least I should have did this in a separate commit
|
Sure I can wait for one week |
|
Also usually I don't care much about the granularity of commits unless many (I mean, > 4) people are involved in the development, but I'd go for smaller commits if you prefer so |
I'd rather have the ability to revert a single commit or PR merge commit to fix something rather than having to understand everything behind the changes. I don't care if everything else is a single commit if it is for a single purpose, regardless of how many files it spans. (e.g. adding a new example app). |
2 participants
This is the app I use mainly for outreach events, so I think it's worth placing it in the main repo.