Skip to content

Use "pack file" instead of "data file"#2885

Merged
MichaelEischer merged 1 commit intorestic:masterfrom
aawsome:packfiles
Aug 16, 2020
Merged

Use "pack file" instead of "data file"#2885
MichaelEischer merged 1 commit intorestic:masterfrom
aawsome:packfiles

Conversation

@aawsome
Copy link
Copy Markdown
Contributor

@aawsome aawsome commented Aug 12, 2020

What does this PR change? What problem does it solve?

Clarify code and documentation by using "pack file" or "pack" everywhere. In some places "data file" was used. This was a bit irritating as there are also data and tree blobs and pack which contain only data blobs or only tree blobs, respectively.

Was the change discussed in an issue or in the forum before?

closes #2877

Checklist

  • I have read the Contribution Guidelines
  • I have enabled maintainer edits for this PR
  • I have not added tests for all changes in this PR, but tests are changed accordingly
  • I have not added documentation for the changes (in the manual) but changed the docu accordingly
  • There's a new file in changelog/unreleased/ that describes the changes for our users (template here)
  • I have run gofmt on the code in all commits
  • All commit messages are formatted in the same style as the other commits in the repo
  • I'm done, this Pull Request is ready for review

- changed variable names, especially changed DataFile into PackFile
- changed in some comments
- always use "pack file" in docu
@aawsome
Copy link
Copy Markdown
Contributor Author

aawsome commented Aug 12, 2020

The change is also needed within #2718 - I used "data files" there and were wondering if that was confusing...

@aawsome aawsome mentioned this pull request Aug 12, 2020
@rawtaz
Copy link
Copy Markdown
Contributor

rawtaz commented Aug 14, 2020

I wonder if there are any or many PRs that will have to be updated if we merge this one first? If so, it might be best to put this on ice until after merging the other ones.

@aawsome
Copy link
Copy Markdown
Contributor Author

aawsome commented Aug 14, 2020

I assume that this doesn't affect many PRs directly, but may produce easy-to-solve merge conflicts if "nearby" lines are changed. From my PRs - as far as I can see - only #2718 and #2543 should be affected.

I would argue that this PR could be used immediately as potential merge conflicts of other PRs will not arise or be easy to solve.
On the other hand, it won't be much to fix for this PR if this really doesn't interfere with other PRs - and if there are merge conflicts, they will be also easy to fix here. When postponing this, we should just make sure that all tests still pass and we do not forget a "data file" sneaked in by another PR 😉

Copy link
Copy Markdown
Member

@MichaelEischer MichaelEischer left a comment

Choose a reason for hiding this comment

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

LGTM. I've thought a bit about whether to merge this now or later on. Besides a few comment and doc changes everything else will lead to a compiler error if the name change is lost during a merge. The few merge conflicts that could arise should also be rather simple to resolve. So I think it's preferential to merge this PR just now rather than wait.

@MichaelEischer MichaelEischer merged commit 0fed6a8 into restic:master Aug 16, 2020
This was referenced Aug 16, 2020
@aawsome aawsome deleted the packfiles branch August 31, 2020 12:16
mfrischknecht pushed a commit to mfrischknecht/restic that referenced this pull request Jun 14, 2022
- changed variable names, especially changed DataFile into PackFile
- changed in some comments
- always use "pack file" in docu
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.

Pack(File) or DataFile?

3 participants