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

Stanza.WriteTo: Sort extra fields alphabetically #803

Merged
merged 4 commits into from
Jan 25, 2019

Conversation

wmanley
Copy link
Contributor

@wmanley wmanley commented Jan 8, 2019

Description of the Change

This makes the output of Stanza.WriteTo deterministic. This is important to me as I am using Packages index files as a kind of lockfile and committing it to my git repository. Without this we get a lot of noise in the diff whenever the file is regenerated because go randomises map iteration order.

Checklist

  • unit-test added (if change is algorithm)
  • functional test added/updated (if change is functional)
  • man page updated (if applicable) - N/A
  • bash completion updated (if applicable) - N/A
  • documentation updated - N/A
  • author name in AUTHORS

This makes the output deterministic.  This is important to me as I am
using `Packages` index files as a kind of lockfile and committing it
to my git repository.  Without this we get a lot of noise in the diff
whenever the file is regenerated because
[go randomises map iteration order][1].

[1]: https://nathanleclaire.com/blog/2014/04/27/a-surprising-feature-of-golang-that-colored-me-impressed/
My measly contribution hardly merits it but it's a requirement in
`CONTRIBUTING.md`.
@wmanley
Copy link
Contributor Author

wmanley commented Jan 8, 2019

I've not yet written a test for this change. I was hoping to expand on existing tests for canonicalOrder but I've not found them. I'm very much a golang novice but if someone can point me to the right place I'm sure I can figure it out. In theory the code should be more testable after this change as it's deterministic.

@wmanley
Copy link
Contributor Author

wmanley commented Jan 9, 2019

Build passed on Travis, but seems to have failed to report the status back to github.

Copy link
Contributor

@smira smira left a comment

Choose a reason for hiding this comment

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

👍 lgtm

@smira
Copy link
Contributor

smira commented Jan 19, 2019

I've submitted #807 which fixes linting errors (should allow this PR to build).

@codecov
Copy link

codecov bot commented Jan 21, 2019

Codecov Report

Merging #803 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #803      +/-   ##
==========================================
- Coverage   64.11%   64.09%   -0.02%     
==========================================
  Files          51       51              
  Lines        6501     6512      +11     
==========================================
+ Hits         4168     4174       +6     
- Misses       1825     1831       +6     
+ Partials      508      507       -1
Impacted Files Coverage Δ
deb/format.go 89.23% <100%> (+0.52%) ⬆️
query/lex.go 84.37% <0%> (-2.73%) ⬇️
deb/remote.go 62.74% <0%> (-0.14%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 50f8cfb...fd99ae0. Read the comment docs.

@codecov
Copy link

codecov bot commented Jan 21, 2019

Codecov Report

Merging #803 into master will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #803      +/-   ##
==========================================
+ Coverage   64.19%   64.23%   +0.03%     
==========================================
  Files          51       51              
  Lines        6508     6514       +6     
==========================================
+ Hits         4178     4184       +6     
  Misses       1825     1825              
  Partials      505      505
Impacted Files Coverage Δ
deb/format.go 89.23% <100%> (+0.52%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 152b3ca...89537b1. Read the comment docs.

@smira smira added this to the 1.4.0 milestone Jan 24, 2019
@smira smira merged commit e2d6a53 into aptly-dev:master Jan 25, 2019
@drothlis drothlis deleted the deterministic-stanza-WriteTo branch May 24, 2022 07:27
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