Skip to content

UI forms as JSON#155

Merged
brtietz merged 20 commits into
patchfrom
SAM_504
Feb 3, 2023
Merged

UI forms as JSON#155
brtietz merged 20 commits into
patchfrom
SAM_504

Conversation

@sjanzou

@sjanzou sjanzou commented Jan 30, 2023

Copy link
Copy Markdown
Collaborator

Associated pull requests in SAM and SAM-private

@sjanzou sjanzou self-assigned this Jan 30, 2023
@brtietz

brtietz commented Jan 30, 2023

Copy link
Copy Markdown
Collaborator

Have we checked in with the other teams that use DView if they're ok with picking up SSC as a dependency? I get why we'd want to do this for our workflow (having Wex as a dependency of SSC seems worse), but it might be worth developing an alternative build process so Wex & DView can be built without SSC. Would the RapidJSON path as an environment variable do the trick?

Apologies if I missed a discussion about this during a meeting.

@sjanzou

sjanzou commented Jan 30, 2023

Copy link
Copy Markdown
Collaborator Author

Have we checked in with the other teams that use DView if they're ok with picking up SSC as a dependency? I get why we'd want to do this for our workflow (having Wex as a dependency of SSC seems worse), but it might be worth developing an alternative build process so Wex & DView can be built without SSC. Would the RapidJSON path as an environment variable do the trick?

Apologies if I missed a discussion about this during a meeting.

The dependency is for rapid json only…in utility.cpp and uniform.cpp.

alternatives are to place rapid json (one header file in wex, too - then no dependencies either way.

If rapidjson was in wex only, then ssc would be dependent on wex….

Comment thread CMakeLists.txt Outdated
#####################################################################################################################

# for rapidjson folder in ssc
include_directories($ENV{SSCDIR})

@brtietz brtietz Jan 30, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Would an environment variable RAPIDJSONDIR work? For SAM developers, this could be set to the SSC directory. Developers without SSC could set it to some other directory with rapidjson. (this might be a good item for the meeting agenda tomorrow)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Would an environment variable RAPIDJSONDIR work? For SAM developers, this could be set to the SSC directory. Developers without SSC could set it to some other directory with rapidjson. (this might be a good item for the meeting agenda tomorrow)

Updated and working in wex, SAM and SAM-private.

@sjanzou sjanzou requested a review from brtietz January 31, 2023 10:41

@brtietz brtietz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This built successfully for me this morning, thanks for updating the environment variables!

We'll need a dedicated PR to merge this to develop that changes the target branch in the CI scripts (after the patch merge).

@sjanzou sjanzou requested a review from brtietz February 3, 2023 10:29

@brtietz brtietz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is great, thank you!

@brtietz brtietz merged commit 7476aac into patch Feb 3, 2023
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