feat: add WithFiles to Directory and Container #6556
Conversation
Since this is a core change you only run changie in the root of the repo, not the sdks. |
Yeah, I was wondering why you didn't 🙂 |
tomasmota
force-pushed
the
with-files
branch
3 times, most recently
from
a773d0a
to
b94e2c7
Compare
|
refactored and added the changeset :) |
|
|
||
| var err error | ||
| for _, file := range src { | ||
| dir, err = dir.WithFile( |
There was a problem hiding this comment.
While this is fine today, I think we could potentially invert this - have WithFile call WithFiles.
We could have mergeStates take multiple source inputs, which would potentially be much more efficient (translating to one MergeOp instead of lots of individual ones). That said, this is a trickier optimization (and I think is also somewhat blocked on #6073).
Just a note, no action required here (but we could potentially leave a TODO comment for follow-up).
docs/docs-graphql/schema.graphqls
Outdated
| """Identifiers of the files to copy.""" | ||
| files: [FileID!]! |
There was a problem hiding this comment.
| """Identifiers of the files to copy.""" | |
| files: [FileID!]! | |
| """Identifiers of the files to copy.""" | |
| sources: [FileID!]! |
We should mirror the structure of withFile.
withFile➡️withFilessource➡️sources
|
Thanks for the contribution @tomasmota! 🎉 Sorry for the nitpicky-comments, only just got round to reviewing this 😢 |
|
The more thorough the review the more I learn, so nitpick away! I'll rename and add the TODO :) |
Signed-off-by: tomasmota <tomasrebelomota@gmail.com>
|
Thanks @tomasmota! |
Fixes: #6475
The issue mentioned only Directory, but I also added it to Container, not sure that was a good idea. Also, this is my first contribution so I might be missing a few things.
With regards to the sdk changelog, it seems like that is something I should do after creating the PR? go through every sdk and run it?
Also a question: in Directory WithFiles, I wasn't sure whether or not I should have re-used WithFile, some feedback on that would be great :)