-
-
Notifications
You must be signed in to change notification settings - Fork 274
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
Reduce code differences between HDF5 1.8 and 1.10 #38
Conversation
Comments, whitespace Simple init and if block brackets. Minimal code changes limited to return value and spelling
autom4te.cache/requests
Outdated
@@ -0,0 +1,252 @@ | |||
# This file was generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If autogen.sh is added, we will also need a .gitignore file, which will ignore the files in autom4te.cache and others, notably Makefile.ins if we're going to full autogen.sh mode. Otherwise I'm not sure we should check in the Makefile.in changes for a newer version of automake which will revert to the older version when regenerated on jelly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forgot to remove that!
OR
should we add it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We decided that the 1.8 branch would not get the autotools changes that went to what became 1.10. autogen.sh and the updated configure options should not be propagated to 1.8 without more discussion.
c++/test/Makefile.in
Outdated
-rm -f ./$(DEPDIR)/tobject.Po | ||
-rm -f ./$(DEPDIR)/trefer.Po | ||
-rm -f ./$(DEPDIR)/ttypes.Po | ||
-rm -f ./$(DEPDIR)/tvlstr.Po |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just curious, why are these listed individual instead of using wildcard? Should I do something differently with the files?
Allen used qutogen.sh on his fedora system because the newer autotools there failed trying to generate trace files with bin/reconfigure. I’ve run bin/reconfigure on jelly and the Makefil.ins will be unchanged. I’ll add it to his PR#38 shortly. That doesn’t answer your question, but these additions will go away.
Larry
From: bmribler <[email protected]>
Sent: Thursday, October 15, 2020 2:24 PM
To: HDFGroup/hdf5 <[email protected]>
Cc: Larry Knox <[email protected]>; Review requested <[email protected]>
Subject: Re: [HDFGroup/hdf5] Hdf5 1 8 (#38)
@bmribler commented on this pull request.
________________________________
In c++/test/Makefile.in<#38 (comment)>:
- -rm -rf ./$(DEPDIR)
+ -rm -f ./$(DEPDIR)/dsets.Po
+ -rm -f ./$(DEPDIR)/h5cpputil.Po
+ -rm -f ./$(DEPDIR)/tarray.Po
+ -rm -f ./$(DEPDIR)/tattr.Po
+ -rm -f ./$(DEPDIR)/tcompound.Po
+ -rm -f ./$(DEPDIR)/tdspl.Po
+ -rm -f ./$(DEPDIR)/testhdf5.Po
+ -rm -f ./$(DEPDIR)/tfile.Po
+ -rm -f ./$(DEPDIR)/tfilter.Po
+ -rm -f ./$(DEPDIR)/th5s.Po
+ -rm -f ./$(DEPDIR)/tlinks.Po
+ -rm -f ./$(DEPDIR)/tobject.Po
+ -rm -f ./$(DEPDIR)/trefer.Po
+ -rm -f ./$(DEPDIR)/ttypes.Po
+ -rm -f ./$(DEPDIR)/tvlstr.Po
I'm just curious, why are these listed individual instead of using wildcard? Should I do something differently with the files?
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub<#38 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ADGMWCVYLRAMSSMRTVSJIATSK5D53ANCNFSM4SR3VH4A>.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to fix the autotools issues before this can be approved.
autotools reverted and other fixes for VFD options |
There is way too much stuff going on in this PR for me to do anything other than blindly click the 'approve' button, especially given how bad github is as a code review tool. Problems:
|
Autotools files have been restored to previous state; bin/reconfigure runs on jelly, changing only 2 trace lines in src/H5FDhdfs.c. |
* Integrate MT ID test with testframe testing framework * Add environment variable to testframe framework for max. threads Adds an environment variable to make it easier for CMake CI to specify the maximum number of threads that multi-threaded tests can spawn in addition to the main thread * Update for const changes and handle errors at coarse granularity --------- Co-authored-by: Matt L <[email protected]>
Mostly comments and whitespace.
Other changes are initialization statements (includes hid_t H5I_INVALID return values on errors) and simple brackets.
The goal was to reduce diffs between 1.10 and 1.8.
The FD and TS files had more extensive changes to reorder the functions to more closely match the order of those in 1.10.