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

Move json reader where it is used #948

Merged
merged 6 commits into from
Nov 26, 2024
Merged

Conversation

JasonMarechal25
Copy link
Contributor

@JasonMarechal25 JasonMarechal25 commented Oct 14, 2024

Json reader is specifically made for and used by StudyUpdater

Base automatically changed from feature/clean_cmake to develop October 16, 2024 14:06
An error occurred while trying to automatically change base from feature/clean_cmake to develop October 16, 2024 14:06
@JasonMarechal25 JasonMarechal25 force-pushed the feature/move_json_reader branch from e1557e7 to 108fcfb Compare October 16, 2024 14:44
@JasonMarechal25 JasonMarechal25 marked this pull request as ready for review October 16, 2024 14:51
@pull-request-size pull-request-size bot added size/L and removed size/M labels Oct 16, 2024
@JasonMarechal25
Copy link
Contributor Author

This pull request includes several changes to the CMakeLists and source files to reorganize and update the JsonXpansionReader component. The most important changes involve moving the JsonXpansionReader from the helpers module to the study-updater module, updating include paths, and adjusting related tests.

Reorganization and Updates:

  • File Moves and Include Path Updates:

    • Moved JsonXpansionReader.cpp from src/cpp/helpers to src/cpp/study-updater and updated its include paths accordingly. (src/cpp/study-updater/JsonXpansionReader.cpp, src/cpp/study-updater/LinkParametersCSVOverwriter.cpp, src/cpp/study-updater/StudyUpdater.cpp) [1] [2] [3]
  • CMakeLists Adjustments:

    • Removed JsonXpansionReader.cpp from src/cpp/helpers/CMakeLists.txt and added it to src/cpp/study-updater/CMakeLists.txt. (src/cpp/helpers/CMakeLists.txt, src/cpp/study-updater/CMakeLists.txt) [1] [2]
    • Updated target_include_directories to include private headers for study_updater_test. (src/cpp/study-updater/CMakeLists.txt)

Testing Adjustments:

  • Test File Reorganization:
    • Removed JsonXpansionReaderTest.cc from helpers_test and added it to study_updater_test. (tests/cpp/helpers/CMakeLists.txt, tests/cpp/study_updater/CMakeLists.txt) [1] [2]
    • Updated JsonXpansionReaderTest to use new paths and included it in the study_updater_test executable. (tests/cpp/helpers/JsonXpansionReaderTest.cc, tests/cpp/study_updater/JsonXpansionReaderTest.cc) [1] [2]

Removal of Redundant Entries:

  • Redundant Source and Header Removals:
    • Removed redundant entries for LinkParametersCSVOverwriter.cpp and StudyUpdateStrategy.cpp from src/cpp/study-updater/CMakeLists.txt. (src/cpp/study-updater/CMakeLists.txt)

Test Configuration:

  • Test Directory Addition:
    • Added study_updater subdirectory to the test configuration. (tests/cpp/CMakeLists.txt)

These changes are aimed at better organizing the codebase and ensuring that the JsonXpansionReader component and its related tests are correctly placed within the study-updater module.

@JasonMarechal25 JasonMarechal25 marked this pull request as draft October 17, 2024 07:51
@JasonMarechal25 JasonMarechal25 force-pushed the feature/move_json_reader branch from ea5a496 to f301714 Compare October 17, 2024 12:58
@pet-mit pet-mit marked this pull request as ready for review November 21, 2024 15:00
@pet-mit pet-mit requested a review from a-zakir November 21, 2024 15:00
@a-zakir a-zakir merged commit b714503 into develop Nov 26, 2024
25 checks passed
@a-zakir a-zakir deleted the feature/move_json_reader branch November 26, 2024 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants