Team-4-Implementation-Final-2

[waynewangyuxuan]

Shadowdash

##This is the updated version based on Dec 11's comment from @kyotov on previous submission PR #24.
0. Graph Building
This is branch is a experimental feature that deconstrucsts and demonstrates how Ninja create it graphs from buiding a simple new-graph/hello_world.cc from scratch.
Do the following:

cd new-graph
./easybuild.sh

Or your can do it manually:
a. Before building simulate-ninja-graph, you need to compile and generate a shared object that includes partial functionality (functions) of Ninja. You can refer to lines 41 to 67 in new-graph/easybuild.sh to find the .cc files that need to be included.
b. After generating the shared object, compile the new-graph/simulate-ninja-graph.cc file and link it with the previously built shared object.
c. Once completed, run ./simulate-ninja-graph and pass hello_world as an argument (./simulate-ninja-graph hello_world). This program will look for a new-graph/hello_world.cc file in the current folder and compile it.
d. After the compilation, an executable file named new-graph/hello_world will be created, which you can run.
(You can modify easybuild.sh based on your OS and compiler to simplify this process.)

[Yaxuan-w]

Collaborate with Qianxi Chen @qianxichen233

Change Granularity

Changes are in one commit, which aligned with our requirement.

Code Functionality

The code meets the HW3 requirements. Logic is consistent and flows well

Readability & Structure

Variable, function names, and file names are descriptive. However, that would be great to distinguish modifications from original code either in code level or in documentations in ninja.h.

Standards Compatibility

We've noticed that there're changes of code style, and we're curious what you're using for formatting. Is there any automation tool you used..?

It's really great for adding hello-world as a new feature API to ninja and achieved desired API exposure!!

Testing

Test cover expected scenarios. We have one minor suggestion: if there're more than one line code for testing in config.yml, that would make more sense to put all command lines into a test script, then config.yml could call it by only one line. This would be super helpful in large scaled testing scenarios.

[Yaxuan-w]

That would be great to have one or two sentences that explains the reason of modification here.

We're curious why the code style of this file changed a lot too..

[Yaxuan-w]

Suggestion-1:

We think header file should only contain function prototypes and actual function implementation should be put into .cc file, because putting actual implementation in header file would case larger binary sizes / symbol redefinition / longer compilation time... Maybe that would be more preferable to place implementation details into ninja.cc

Ref: https://www.quora.com/Why-could-we-not-implement-definition-in-a-header-file

This is only a suggestion. Including implementations in header file will make development process much easier (we also did this way during our development phase), but we believe that would be great to follow standard to avoid potential issues in final version :D

Suggestion-2:

That would be great if modifications can be distinguished from original code (the one from ninja.cc) for more readability