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

[web] Support config file #2611

Merged
merged 2 commits into from
May 13, 2020
Merged

Conversation

csordasmarton
Copy link
Contributor

Closes #427

docs/web/user_guide.md Outdated Show resolved Hide resolved
docs/web/user_guide.md Outdated Show resolved Hide resolved
"emplaced as command line arguments. The format "
"of configuration file is: "
"{"
" \"enabled\": true,"
Copy link
Contributor

Choose a reason for hiding this comment

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

To me this enabled value is a bit confusing, if I want to use a config file I set it in the command line, if not I do not set it.
But why would I set it in the command line and than disable it in the config file?
@bruntib @dkrupp what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The .codechecker.passwords.json and the server_config.json look the same: both have an option to turn off or on the feature. I wanted to be more consistent with these features. By the way the analyzer --config option works the same so if we would like to change this behaviour we can do it in another pull request. Maybe we can create an issue for this.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've created a separate issue to discuss it #2654

web/client/codechecker_client/cmd/store.py Outdated Show resolved Hide resolved
web/tests/functional/config/__init__.py Outdated Show resolved Hide resolved
web/tests/functional/config/__init__.py Outdated Show resolved Hide resolved
web/tests/libtest/codechecker.py Show resolved Hide resolved
web/tests/functional/config/__init__.py Outdated Show resolved Hide resolved
@@ -0,0 +1,70 @@
# coding=utf-8
Copy link
Contributor

Choose a reason for hiding this comment

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

the coding part is not needed by default in py3 every file should be utf-8

"emplaced as command line arguments. The format "
"of configuration file is: "
"{"
" \"enabled\": true,"
Copy link
Contributor

Choose a reason for hiding this comment

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

I've created a separate issue to discuss it #2654

Copy link
Contributor

@gyorb gyorb left a comment

Choose a reason for hiding this comment

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

It LGTM but first we should reach a decision in #2654 to see if the enabled configuration option should be kept or not.

@csordasmarton
Copy link
Contributor Author

@gyorgy I already commented on that issue. We can also remove it later if we do not want to keep that.

@csordasmarton csordasmarton force-pushed the web_config_file branch 2 times, most recently from 25707a6 to fafa224 Compare May 7, 2020 13:56
@csordasmarton
Copy link
Contributor Author

@gyorb Okay, I have removed this enabled key from this configuration file and from the source code.

{
"store": [
"--name=run_name",
"--tag=my_tag"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"--tag=my_tag"
"--tag=my_tag",

A comma is missing, the json is invalid without it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, thanks!

web/client/codechecker_client/cmd/store.py Outdated Show resolved Hide resolved
@dkrupp dkrupp mentioned this pull request May 12, 2020
2 tasks
@gyorb
Copy link
Contributor

gyorb commented May 13, 2020

Codacy Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- web/tests/functional/cli_config/__init__.py  1
- web/tests/functional/cli_config/test_store_config.py  3
- web/tests/functional/cli_config/test_server_config.py  3
         

Clones added
============
- web/client/codechecker_client/cmd/store.py  1
- web/tests/functional/cli_config/test_store_config.py  1
- web/tests/functional/cli_config/test_server_config.py  7
- web/server/codechecker_server/cmd/server.py  1
         

See the complete overview on Codacy

@gyorb gyorb merged commit 0f58661 into Ericsson:master May 13, 2020
@csordasmarton csordasmarton deleted the web_config_file branch July 14, 2020 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation 📖 Changes to documentation. enhancement 🌟 server 🖥️ test ☑️ Adding or refactoring tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CodeChecker config file support
2 participants