Skip to content

Conversation

Dennis40816
Copy link
Contributor

During testing, an error occurred in the process of executing 'trace-03-ops.cmd'. Upon investigation, it was found that the issue was caused by an incorrect implementation of 'q_reverse'. However, this function was not explicitly listed in the trace command descriptions, which could reduce debugging efficiency. Therefore, this PR updates the descriptions in the trace command files.

This PR does not modify any test data.

@jserv
Copy link
Contributor

jserv commented Mar 1, 2025

I would like to ask @lumynou5 for confirmation.

@jserv jserv requested a review from eecheng87 March 1, 2025 14:21
@eecheng87
Copy link
Collaborator

The change is OK for me, but I prefer minimal changes to the purpose.

For example, just append the missing description to the original first line.
Also, I think '# Test of operations - *' is redundant since the testing index can be inferred from the file name.

@Dennis40816
Copy link
Contributor Author

The change is OK for me, but I prefer minimal changes to the purpose.

For example, just append the missing description to the original first line. Also, I think '# Test of operations - *' is redundant since the testing index can be inferred from the file name.

There were several reasons for considering the addition of a dedicated line listing the functions required for each test:

  1. To make function requirements clearer and more consistent across all test files.
  2. To avoid cluttering the test title. Placing all functions directly after the title could make it excessively long or difficult to read, especially since the title lengths vary. In my own testing, listing all functions immediately after the title made it inconvenient to read.

I completely agree that # Test of operations - * is ineffective. I will soon update the format by moving the required function names to the first line and removing the operation test numbering.

The revised format will likely be:
Original topic: 'q_new', 'q_insert_head', 'q_remove_head'
If the original topic already contains some functions, the missing ones will be added within the topic itself:
Example: Test of insert_head and remove_headTest of 'q_new', 'q_insert_head', 'q_remove_head'.

If you have any suggestions, feel free to share them.

@Dennis40816 Dennis40816 force-pushed the refine-trace-cmd branch 2 times, most recently from c251922 to 9a279be Compare March 1, 2025 15:24
Updated the trace command files to specify required operations
explicitly. Improved documentation without modifying test data.

Change-Id: If8ef44171bb5cc3cfee812d74cc33d55f6f1320b
@jserv jserv requested a review from lumynou5 March 2, 2025 03:44
@jserv jserv merged commit c0cefe9 into sysprog21:master Mar 2, 2025
1 of 2 checks passed
@jserv
Copy link
Contributor

jserv commented Mar 2, 2025

Thank @Dennis40816 for contributing!

@Dennis40816 Dennis40816 deleted the refine-trace-cmd branch March 6, 2025 16:05
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