Skip to content
This repository has been archived by the owner on Jun 16, 2024. It is now read-only.

fix: remove suffix #20

Merged
merged 1 commit into from
Dec 26, 2019
Merged

fix: remove suffix #20

merged 1 commit into from
Dec 26, 2019

Conversation

juanpicado
Copy link
Member

add new test scenarios

add new test scenarios
Copy link
Member

@anikethsaha anikethsaha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥇

@juanpicado juanpicado merged commit 28df490 into master Dec 26, 2019
@juanpicado juanpicado deleted the test-add-scenarios branch December 26, 2019 07:11
@DanielRuf
Copy link

Might be better to update the lockfile in a separate commit. This would make the diffs much smaller and better to review locally =)

@anikethsaha
Copy link
Member

yeah true, but when we make changes related to dep like version changing or dep remove/add, isn't it required to use the committed lock file in order to build the same env?

Having it in separate commit is useful cause it makes things kind opt-in or optional

@DanielRuf
Copy link

yeah true, but when we make changes related to dep like version changing or dep remove/add, isn't it required to use the committed lock file in order to build the same env?

Make two commits and push after these two.

@DanielRuf
Copy link

DanielRuf commented Dec 26, 2019

Only the pushes trigger CI, also see how we did it at https://github.com/bitExpert/mattermost-client-node/commits/develop

This also produces smaller commit objects / blobs in most cases and better compression in Git itself.

@anikethsaha
Copy link
Member

Make two commits and push after these two.

lock file commit should be the last one?

@DanielRuf
Copy link

lock file commit should be the last one?

Exactly.

@anikethsaha
Copy link
Member

lock file commit should be the last one?

Exactly.

yea...make sense 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants