Skip to content

Feature/statistics update#524

Merged
pdschubert merged 13 commits intodevelopmentfrom
feature/statisticsUpdate
Oct 26, 2022
Merged

Feature/statistics update#524
pdschubert merged 13 commits intodevelopmentfrom
feature/statisticsUpdate

Conversation

@janniclas
Copy link
Copy Markdown
Member

This merge request contains

  • typo fixes statistic -> statistics
  • added the module name to the exported json statistics, to easily identify the result files of multiple analyses.
  • Update to the Docker development env file to correctly set the PATH variable
  • Updates to the Dockerfile to let it build way faster ( we don't build LLVM in the image, because this image is intended to run analyses in release mode and from my understanding we don't need the custom build version of LLVM for this). This update was inspired from the test setup from PhASAR in the GitHub actions.

@janniclas janniclas requested a review from fabianbs96 October 17, 2022 08:51
@janniclas
Copy link
Copy Markdown
Member Author

Extended the request with a new GitHub action to build the docker image and publish it to GitHubs container registry. This can greatly ease the initial adoption of PhASAR imho.
It would be even better if we also push the image to the official docker registry, if you think this is desirable just reach out to me and I can walk you through the necessary steps @pdschubert .

Copy link
Copy Markdown
Member

@fabianbs96 fabianbs96 left a comment

Choose a reason for hiding this comment

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

The C++ part of the PR looks good to me; the docker.yml I cannot verify due to missing expertise

@pdschubert pdschubert self-requested a review October 26, 2022 07:06
@pdschubert
Copy link
Copy Markdown
Member

Hey @vulder can you tell why the pre-commit fails here? And if so, how to fix it?

Thanks!
Philipp

@vulder
Copy link
Copy Markdown
Collaborator

vulder commented Oct 26, 2022

@pdschubert The hook most often tells you the issue: trim trailing whitespace.................................................Failed
After the docker line are trailing whitespaces, remove them and the check will be happy again 😀

Comment thread .github/workflows/docker.yml Outdated
@pdschubert pdschubert merged commit 42617f1 into development Oct 26, 2022
@pdschubert pdschubert deleted the feature/statisticsUpdate branch October 26, 2022 09:19
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.

4 participants