Skip to content

Conversation

@heshamalmosawi
Copy link
Contributor

Why?

  1. Job Control Project
    This project has unclear and inconsistent requirements to the audit file, as well as having redundant audit files. This fix attempt to solve some of the inconsistencies, however in the solution overview will mention some of the unclear gaps still remaining in the requirements and left up to the maintainers to fix as they see fit.
  2. Scripting Project
    This project has inconsistent requirements, as well as having redundant audit files.

Implementation Details

  1. Scripting Project
  • This project has two audit files, one of them was removed and kept the audit/README.md file to stay consistent with other projects.
  • The project requirement as well as first question has been changed to Rust only to stay consistent with the main 0-shell project.
  1. Job Control Project
  • The biggest issue with this project was clarity in requirements, The requirements have been re-written to be clearer, requiring everything that is in the audit file. Two unclear points still remaining are the use of python, for this i have suggested allowing Command::new to spawn a python process. The other unclear point is the use of 2>1 >/dev/null, I was not sure about this point, as it was not required in the main project, so i left it up to the 01 staff to decide how to introduce this, as well as if my python fix is sufficient.
  • This project has two audit files, one of them was moved to audit/README.md and removed the one in the root to stay consistent with other projects.
  • The project requirement as well as first question has been changed to Rust only to stay consistent with the main 0-shell project.

@Oumaimafisaoui Oumaimafisaoui requested a review from pedrodesu July 11, 2025 08:36
@pedrodesu
Copy link
Contributor

Hello @heshamalmosawi! We appreciate your extensive PR. Your contributions are very much appreciated!

In general all the changes you presented seem good and well justified. In regards to the python situation - You make a great point. The point of the python command is to see that the process is spawned and then terminated, while the sleep runs.
Fortunately we can easily solve this with any command with the same usage as python - such as cat, which we tell them to implement. As such, I suggest you remove back the "allowing python" part on the job-control README, and changing the occurrences of python with cat inside of the job-control's audit file. Reinforcing to the student that cat should also have the expected behavior without arguments (echo back any input given) is also probably a good idea.
With these changes done everything should be sound and I can approve your contribution.

Once again, thank you so much for your time! We appreciate it.

@heshamalmosawi
Copy link
Contributor Author

heshamalmosawi commented Jul 15, 2025

Hello @pedrodesu,
Thanks for the feedback! I have updated the job-control project to use cat instead of python. To be consistent, I have added the cat no-argument mode into the main 0-shell project, to make sure that students have this option implemented, prior to going to the optional.

Although, there is still a gap with making the 2>1 & >/dev/null redirections a requirement, since I am not sure how to explain them, or what their scope is. My personal suggestion is for this use case, replacing the combination of both with a single custom alias (pre-defined name in the requirements) for this project that discards stdout, to maintain clarity & straight-forwardness in the project requirements. However your expertise and knowledge will probably more knowledgeable.

Please review the changes and feel free to edit, or have additions, or let me know so I can add those changes.

@heshamalmosawi heshamalmosawi changed the title [EXTERNAL] docs(0-shell optionals): refactoring of 0-shell optional docs [EXTERNAL] docs(0-shell + optionals): refactoring of 0-shell & its optionals' docs Jul 16, 2025
@pedrodesu
Copy link
Contributor

In regards to the 2>1 & >/dev/null, could you please clarify a bit better what you meant? As the subject states > is not fundamental to implement. If you do the bonus and wish to implement >, it should have the same basic behavior as in bash: Redirecting stdout to something else. Whether it is another command, /dev/null or any other file. One could also implement 2> with this same mentality, although it is not required, even in the bonus. (By the way, 2> does the exact same thing as >, but instead of redirecting stdout it redirects stderr)

@heshamalmosawi
Copy link
Contributor Author

In regards to the , could you please clarify a bit better what you meant? As the subject states > is not fundamental to implement. If you do the bonus and wish to implement >, it should have the same basic behavior as in bash: Redirecting stdout to something else. Whether it is another command, /dev/null or any other file. One could also implement 2> with this same mentality, although it is not required, even in the bonus. (By the way, 2> does the exact same thing as >, but instead of redirecting stdout it redirects stderr)

Hello @pedrodesu,
The 2>1 & >/dev/null part is required in the audit question as a core requirement (first question of the audit). My suggestion was to add a requirement to this project of making an alias equivalent of running 2>1 >/dev/null as a whole, simplifying the implementation for the student.

For instance, instead of the first question being run ls -lRr / 2>1 >/dev/null &, we could instruct them to run ls -lRr / <alias_name> &, where this alias replicates the behavior of discarding stdout. This question is necessary because if stdout is not discarded, the student will not be able to achieve the required outcome. However, I am not sure if there is any other solution or if the 01 team would prefer this, so I left it for your interpretation.

As I'm writing this, I suppose that having another sleep 5000 & or such would also suffice in this case, to keep redirecting still a bonus feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants