Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
✨ [#1910] Add first NL-DesignSystem components #909
✨ [#1910] Add first NL-DesignSystem components #909
Changes from all commits
f8a5354
70a1a45
d396036
c61a947
1d2dee6
bbcc40f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how often this repo will be updated, but I think it would be a good idea to pin this to a specific tag. Otherwise every pipeline of Open Inwoner will always pull in the
master
branch of the package's repo.We could probably do something like this: https://stackoverflow.com/a/21699307 (also makes us skip any history, since we're not going to use that in the image anyway)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've just created a Tag (v0.0.1) but I wonder: if this will actually still be updated a lot, isn't it better to keep on pulling the Main branch - in these beginning stages? Every commit means a new Tag since they are big changes every time (I think).
In a far(?) future that repo could become the single source of truth (when they will also be used by the designers).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I already had to update it to version 0.0.2... :-D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could let it point to the main branch if it will be modified a lot, though I am generally wary of not pinning these kinds of dependencies, because I like to know what versions of dependencies I have for a specific image and not have potential unwanted changes be included automatically. If @alextreme approves to use master then it's fine by me.
You could add
--depth 1
togit clone
to avoid pulling in the history (https://git-scm.com/docs/git-clone#Documentation/git-clone.txt---depthltdepthgt)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I already turned it into this "
RUN git clone --depth 1 --branch 0.0.2 https://github.com/maykinmedia/open-inwoner-design-tokens.git
" with the tag, but how will we remember the Tag needs to be updated too? Add to both repo's documentations?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's indeed quite a chore to keep these kinds of relations in-sync between repositories, this is why for maykin-deployment I also avoid pinning to a specific branch (and why I'd prefer to go for a single combined repo, this would avoid the problem altogether).
For now I approve and prefer pulling from main/master instead of a release-branch. I'll set a reminder to re-evaluate this in half a year, if changes have become incidental then we'll pin this again to a release-branch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stevenbal @alextreme So should I remove the clone-from-Tag thing and revert to what it was?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes