Skip to content
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

Update mix.lock after dependecies update #161

Merged
merged 2 commits into from
May 20, 2023

Conversation

nappex
Copy link
Collaborator

@nappex nappex commented Jan 15, 2023

Updated dependecies by mix deps.update by that command mix.lock changed. Before merge consider Issue #160

@nappex
Copy link
Collaborator Author

nappex commented Jan 15, 2023

It seems I did not understand ~> correctly:

~> will never include pre-release versions of its upper bound, regardless of the usage of the :allow_pre option, or whether the operand is a pre-release version. It can also be used to set an upper bound on only the major version part. See the table below for ~> requirements and their corresponding translations.

~> Translation
~> 2.0.0 >= 2.0.0 and < 2.1.0
~> 2.1.2 >= 2.1.2 and < 2.2.0
~> 2.1.3-dev >= 2.1.3-dev and < 2.2.0
~> 2.0 >= 2.0.0 and < 3.0.0
~> 2.1 >= 2.1.0 and < 3.0.0

@nappex
Copy link
Collaborator Author

nappex commented Jan 15, 2023

I suggest close the PR without merge, when i run:

$ mix deps.clean httpoison mox
$ mix deps.get

The mix.lock haven't changed.

@Glutexo
Copy link
Owner

Glutexo commented Jan 16, 2023

The lock file ensures that anybody who clones the project has the same (and assumably working) versions of the dependencies. You can always re-sync your lock file yourself if you want to upgrade. See the discussion here: https://stackoverflow.com/questions/4151495/should-gemfile-lock-be-included-in-gitignore. What do you think?

@Glutexo Glutexo mentioned this pull request Jan 23, 2023
@Glutexo Glutexo self-requested a review January 23, 2023 16:30
@nappex
Copy link
Collaborator Author

nappex commented Feb 5, 2023

Probably, we solve the problem by another approach, issue related to this is #167

Glutexo
Glutexo previously approved these changes May 1, 2023
Copy link
Owner

@Glutexo Glutexo left a comment

Choose a reason for hiding this comment

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

The mix.lock file is ok. Re-generating it by deleting it and running the commands you suggested yields exactly the same file.

$ mix deps.clean httpoison mox
$ mix deps.get

Thus, I consider this bump valid. The tests are passing and manual run of the escript works like a charm.

@Glutexo
Copy link
Owner

Glutexo commented May 4, 2023

@nappex Do you know how to resolve the conflict? Are you willing to do it? Or do you want to rather ditch this PR altogether? I’d prefer to merge it, since it’s easy and two smaller bumps are better than one big.

@Glutexo
Copy link
Owner

Glutexo commented May 8, 2023

@nappex Resolving the conflict in the mix.lock file should be quite simple as the file as a whole doesn't have any fingerprint that needs to be recalculated. All you need to do is merge the lines. Let me know if you need any assistance!

@Glutexo
Copy link
Owner

Glutexo commented May 8, 2023

I've resolved the conflict by keeping the updated versions from this pull request and also adding the new lines from the master branch for floki and its dependency html_entities. After merging, I ran mix deps.get, and everything installed successfully.

I'm leaving the conflict resolution in this pull request for you to handle, so you can gain some experience with it. Once you've completed the process, please let me know, and I will review and approve it.

@nappex
Copy link
Collaborator Author

nappex commented May 16, 2023

The lock file ensures that anybody who clones the project has the same (and assumably working) versions of the dependencies. You can always re-sync your lock file yourself if you want to upgrade. See the discussion here: https://stackoverflow.com/questions/4151495/should-gemfile-lock-be-included-in-gitignore. What do you think?

OK, I found these answers:

1) https://bundler.io/guides/faq.html
Q: Should I commit my Gemfile.lock when writing a gem?

A: Yes, you should commit it. The presence of a Gemfile.lock in a gem’s repository ensures that a fresh checkout of the repository uses the exact same set of dependencies every time. We believe this makes repositories more friendly towards new and existing contributors. Ideally, anyone should be able to clone the repo, run bundle install, and have passing tests. If you don’t check in your Gemfile.lock, new contributors can get different versions of your dependencies, and run into failing tests that they don’t know how to fix.

**2)**https://www.viget.com/articles/bundler-best-practices/
Should you keep your Gemfile.lock in version control? Yes! It's a critical part of getting the most benefit from Bundler. By doing so, you guarantee that every developer working on the project and every deployment of the app will use exactly the same third-party code. How awesome is that?

More people advice to keep lock file in your repo. I am not able to say if we can compare gem and mix.
Because of these above I agree with you to keep this PR and solve the conflicts.

@nappex
Copy link
Collaborator Author

nappex commented May 16, 2023

I solved the conflict @Glutexo. It should be fine now, but diff is little bit strange

@nappex nappex force-pushed the dependecies-update branch from 9acbb99 to f0786a2 Compare May 16, 2023 10:26
@nappex
Copy link
Collaborator Author

nappex commented May 16, 2023

I've also solved strange diff, dependecies have not been in order. @Glutexo
Ready for review!

Copy link
Owner

@Glutexo Glutexo left a comment

Choose a reason for hiding this comment

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

Great job, the conflicts have been resolved and the tests are passing on my machine. Thank you for your patience, even though this work might have seemed unnecessary in the context of the related issue #167, which will overwrite these changes.

@Glutexo Glutexo merged commit dc0b3fc into Glutexo:master May 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants