Revert "fixed reading of REST v2.2 files" #289
Conversation
This reverts commit 050d3bc.
|
This will close PR #288 |
| @@ -242,13 +242,8 @@ inline TClass* GetClassQuick(std::string type) { | |||
| return iter->second; | |||
| } else { | |||
| TClass* cl = TClass::GetClass(type.c_str()); | |||
| if (cl != nullptr && cl->HasDictionary()) { | |||
There was a problem hiding this comment.
@nkx111 Probably the cl->HasDictionary() here is a problem.
|
I think we should see this issue with a different perspective, we should understand why the dictionary is not needed while compiling The changes that you propose triggers the generation of the dictionary, I think that due to the changes that were done before this line was never reached. I think that we should avoid the generation of a dictionary on the fly, so we should understand what is going on with the compilation. I think this issue should be fixed in the |
|
I think the reason why the reverted commit introduces the problem is because the type that has no dictionary is not added to the list, and therefore, it is not automatically compiled on the fly, since there is the new condition I agree that could be a better way to do this, but personally I don't understand deeply all behind reflection. Perhaps we could just have a list of supported types in REST somewhere, and generate the dictionaries at |
This reverts commit 568baeb.
…work into jgalan_fix_rawlib
|
I think that the pipeline passes because we have a data member |
|
I think generating dictionaries on the fly is still the best way to solve this problem. Commit 050d3bc avoids problematic TClass objects being read from v2.2 files, but vetos the real-time generation functionality. I will try to fix it. |
|
@nkx111 I don't understand why creating the required dictionaries at Anyway, shall we merge this PR to fix pipelines at rawlib, then create an issue for further discussion? |
|
Sorry for the delay. Now the file opening problem is fixed from TRestRun side, I think it's OK to revert the changes in TRestReflector. |
|
hi @jgalan, the pipelines are faling due to a certificate issue, can you check it out? |
|
I believe now the pipeline is failing because |
|
Lets merge? |
Since we cannot disable checking for certificates everywhere ( https://github.com/rest-for-physics/framework/pull/289/files#r962688304 |
Co-authored-by: Luis Antonio Obis Aparicio <35803280+lobis@users.noreply.github.com>
This reverts commit 050d3bc.
This should fix rest-for-physics/rawlib#74