Skip to content

Cleanup 01 - #8

Merged
dirkboye merged 4 commits into
dectris-cloud:masterfrom
dirkboye:cleanup-01
Mar 1, 2021
Merged

Cleanup 01#8
dirkboye merged 4 commits into
dectris-cloud:masterfrom
dirkboye:cleanup-01

Conversation

@dirkboye

@dirkboye dirkboye commented Mar 1, 2021

Copy link
Copy Markdown
Contributor

This pullrequest is a first preparation for adding support of HDF5 superblock version 2.

dirkboye added 2 commits March 1, 2021 14:52
we don't want to create a H5Object at address linkInformationOffset
but at the address extracted as uint64 from current file at
linkInformationOffset.

this was merely a typo as the method names for extraction of
values are very similar to cpp types.
…data

make intention of methods more clear and prevent future bugs.
@dirkboye
dirkboye requested a review from kalcutter March 1, 2021 16:10
@kalcutter

Copy link
Copy Markdown
Contributor

I would not reformat any third party code (e.g. bitshuffle, or lz4).

@dirkboye

dirkboye commented Mar 1, 2021

Copy link
Copy Markdown
Contributor Author

will ignore third_party on clang-format!

@kalcutter

Copy link
Copy Markdown
Contributor

Is it possible to leave formatting out of this PR entirely? It does make it harder to review.

@dirkboye

dirkboye commented Mar 1, 2021

Copy link
Copy Markdown
Contributor Author

removed the reformat

@kalcutter kalcutter left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The commit message text "move methods only used by class to private" no longer applies does it?

Comment thread src/dectris/neggia/data/H5HeaderMsg.h
Comment thread src/dectris/neggia/data/H5HeaderMsg.h Outdated
@dirkboye
dirkboye force-pushed the cleanup-01 branch 2 times, most recently from 91f5ae9 to 9cf9bb0 Compare March 1, 2021 19:27
@kalcutter

Copy link
Copy Markdown
Contributor

LGTM

* move methods only used by class to private.

* refactor H5HeaderMsgPreamble to struct H5HeaderMessage
  as this only wraps object and type to the outside.
@dirkboye

dirkboye commented Mar 1, 2021

Copy link
Copy Markdown
Contributor Author

adapted commit message only.

@dirkboye
dirkboye merged commit 29bc241 into dectris-cloud:master Mar 1, 2021
@dirkboye
dirkboye deleted the cleanup-01 branch March 2, 2021 13:52
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