-
Notifications
You must be signed in to change notification settings - Fork 78
Rework example task layout #786
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
Changes from 3 commits
c19b22b
a226569
902fafb
c5fb33e
c1ec2eb
5879ca9
7d7502c
76af06e
7c0d5db
356e6bb
5aac6c3
674221f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please, make sure that all .po files are actually update
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated the translation catalogs with sphinx-intl after regenerating gettext catalogs. docs_gettext, docs_update, and docs_html pass locally. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -107,13 +107,13 @@ Tips for tests | |
|
|
||
| .. code-block:: json | ||
|
|
||
| { "tasks_type": "processes", "tasks": { "mpi": "enabled", "seq": "enabled" } } | ||
| { "tasks": { "mpi": "enabled", "seq": "enabled" } } | ||
|
|
||
| - ``info.json`` — student metadata used in automation (scoreboard, macros): | ||
|
|
||
| .. code-block:: json | ||
|
|
||
| { "student": { "full_name": "Фамилия Имя Отчество", "group_number": "Группа", "task_number": "1" } } | ||
| { "student": { "last_name": "Фамилия", "first_name": "Имя", "middle_name": "Отчество", "group_number": "Группа" } } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you sure you want to change it also right here right now?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. The metadata format is back to full_name + group_number |
||
|
|
||
| Build and local run | ||
| ------------------- | ||
|
|
@@ -188,8 +188,12 @@ Common pitfalls (read before pushing) | |
|
|
||
| Useful examples to reference | ||
| ---------------------------- | ||
| - Processes: ``tasks/example_processes``, ``tasks/example_processes_2``, ``tasks/example_processes_3`` | ||
| - Threads: ``tasks/example_threads`` | ||
| - Unified example: ``tasks/example`` | ||
| - Shared example files: ``tasks/example/settings.json``, ``tasks/example/info.json``, ``tasks/example/common``, ``tasks/example/data`` | ||
| - Threads: ``tasks/example/threads/{seq,omp,tbb,stl,all}`` | ||
| - Processes: ``tasks/example/processes/t1``, ``tasks/example/processes/t2``, ``tasks/example/processes/t3`` | ||
| - Process tasks keep independent ``seq`` and ``mpi`` implementations under each ``tN`` directory. | ||
| - Example reports are tree-shaped: the root report links to section reports and each implementation directory has its own ``report.md``. | ||
|
|
||
| - Work from your fork in a dedicated branch (not ``master``). Branch name must match your task folder. | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -80,7 +80,8 @@ inline std::string GetStringTaskStatus(StatusOfTask status_of_task) { | |
| /// @param settings_file_path Path to the JSON file containing task type strings. | ||
| /// @return Formatted string combining the task type and its corresponding value from the file. | ||
| /// @throws std::runtime_error If the file cannot be opened. | ||
| inline std::string GetStringTaskType(TypeOfTask type_of_task, const std::string &settings_file_path) { | ||
| inline std::string GetStringTaskType(TypeOfTask type_of_task, const std::string &settings_file_path, | ||
| std::string_view settings_task_path = {}) { | ||
| std::ifstream file(settings_file_path); | ||
| if (!file.is_open()) { | ||
| throw std::runtime_error("Failed to open " + settings_file_path); | ||
|
|
@@ -94,8 +95,18 @@ inline std::string GetStringTaskType(TypeOfTask type_of_task, const std::string | |
| return std::string(type_str); | ||
| } | ||
|
|
||
| const auto &tasks = list_settings->at("tasks"); | ||
| return std::string(type_str) + "_" + std::string(tasks.at(std::string(type_str))); | ||
| const auto *settings_node = &list_settings->at("tasks"); | ||
| for (size_t start = 0; start < settings_task_path.size();) { | ||
| const size_t separator = settings_task_path.find('.', start); | ||
| const size_t key_size = separator == std::string_view::npos ? settings_task_path.size() - start : separator - start; | ||
| settings_node = &settings_node->at(std::string(settings_task_path.substr(start, key_size))); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please, make a proper error hanling here instead of using
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. Replaced direct .at() access with explicit key validation and contextual runtime errors, and added a unit test for missing nested settings paths. |
||
| if (separator == std::string_view::npos) { | ||
| break; | ||
| } | ||
| start = separator + 1; | ||
| } | ||
|
|
||
| return std::string(type_str) + "_" + std::string(settings_node->at(std::string(type_str))); | ||
| } | ||
|
|
||
| enum class StateOfTesting : uint8_t { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. The repository has a tracked Windows batch script with CRLF line endings (
scripts/generate_perf_results.bat), while the global EditorConfig rule setsend_of_line = lffor all files. This override resolves that conflict for.bat/.cmdfiles and keeps Windows scripts in their native line-ending format.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it necessary for this patch? I'd even separate docs part at this point as well