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

Excercise n1 #4

Merged
merged 1 commit into from
Dec 14, 2022
Merged

Excercise n1 #4

merged 1 commit into from
Dec 14, 2022

Conversation

sorru94
Copy link
Owner

@sorru94 sorru94 commented Nov 18, 2022

Created an initial implementation that parses the provided test-output file and stores the result into a test-output-parsed file.

@sorru94 sorru94 marked this pull request as draft November 18, 2022 12:51
src/disasm.rs Outdated Show resolved Hide resolved
@sorru94 sorru94 force-pushed the excercise-n1 branch 4 times, most recently from 42e9bee to a547ede Compare November 21, 2022 16:23
src/disasm.rs Outdated Show resolved Hide resolved
src/disasm.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
@sorru94 sorru94 force-pushed the excercise-n1 branch 3 times, most recently from 8ff0d22 to c630aa7 Compare November 22, 2022 11:09
@sorru94 sorru94 marked this pull request as ready for review November 22, 2022 11:38
@sorru94 sorru94 force-pushed the excercise-n1 branch 2 times, most recently from be25c89 to 2bdec75 Compare November 22, 2022 17:28
src/disasm.rs Show resolved Hide resolved
src/disasm.rs Show resolved Hide resolved
src/disasm.rs Show resolved Hide resolved
src/disasm.rs Show resolved Hide resolved
src/disasm.rs Show resolved Hide resolved
src/disasm.rs Show resolved Hide resolved
src/disasm.rs Show resolved Hide resolved
src/disasm.rs Show resolved Hide resolved
src/disasm.rs Show resolved Hide resolved
src/disasm.rs Show resolved Hide resolved
@harlem88
Copy link
Collaborator

The Reuse Compliance Check fails because the following files have no copyright and licensing information:

  • test-output
  • test-output-parsed

consider using the CC0 license for this files.
I think there are a generated file, so you can use a DEP5 file in order to execlude them. Here have an example
https://github.com/edgehog-device-manager/edgehog-device-runtime/blob/main/.reuse/dep5

@sorru94 sorru94 force-pushed the excercise-n1 branch 2 times, most recently from d69e8e6 to 2941d24 Compare November 23, 2022 16:15
src/disasm/section.rs Outdated Show resolved Hide resolved
@sorru94 sorru94 requested a review from bettio November 24, 2022 08:44
src/main.rs Outdated Show resolved Hide resolved
@sorru94 sorru94 force-pushed the excercise-n1 branch 3 times, most recently from 84755c1 to fc5fd44 Compare November 25, 2022 14:46
@bettio
Copy link
Collaborator

bettio commented Nov 28, 2022

Never add huge blobs (such as test-output and test-output-parsed) to git repos, they bloat down the repository, they are harder to manage, and etc.
Let's make smaller reference files that are easier to understand and manipulate.

@bettio
Copy link
Collaborator

bettio commented Nov 28, 2022

It looks like that test-output and test-output-parsed are not referenced from any line of code, so maybe they shouldn't be here.

.reuse/dep5 Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/disasm.rs Outdated Show resolved Hide resolved
src/disasm.rs Show resolved Hide resolved
src/disasm.rs Outdated Show resolved Hide resolved
src/disasm/instruction.rs Outdated Show resolved Hide resolved
src/disasm/section.rs Outdated Show resolved Hide resolved
src/disasm/section.rs Outdated Show resolved Hide resolved
@sorru94
Copy link
Owner Author

sorru94 commented Nov 30, 2022

It looks like that test-output and test-output-parsed are not referenced from any line of code, so maybe they shouldn't be here.

@bettio Yes, I agree with this. I was keeping them versioned because I wanted to ensure I was not changing the behaviour of the parser during development. But now that there are more unit tests (also in the PR of excercise 2) they should be removed.

@sorru94 sorru94 force-pushed the excercise-n1 branch 2 times, most recently from fbc99da to 17a9276 Compare December 1, 2022 13:27
@sorru94 sorru94 mentioned this pull request Dec 1, 2022
Signed-off-by: Simone Orru <simone.orru@secomind.com>
@sorru94 sorru94 merged commit 0f8c537 into main Dec 14, 2022
@sorru94 sorru94 deleted the excercise-n1 branch December 14, 2022 15:32
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