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

Merge "titotix/refactoring" into "jbremer/master" #1

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

Titotix
Copy link

@Titotix Titotix commented Oct 25, 2016

Hey,
I merged our work and will continue further ahead as we discuss jesparza#57.

Titotix added 15 commits May 30, 2016 21:00
export code from bin/peepdf to PDFOutput.py
code from peepdf main script is dispatched around PDFCore, PDFOutput & PDFUtils
The main script peepdf is now a wrapper to this moved code
intro will show the report of the provided file
This was normal behaviour before refactoring
constants file contains all constant variable like project info, author...
getReport is a wrapper to getPeepReport, getPeepJSON & getPeepXML
@jbremer
Copy link
Member

jbremer commented Oct 27, 2016

Cool. Could you elaborate on the exact changes in this PR? I see a lot of code moving around etc.

@jbremer
Copy link
Member

jbremer commented Oct 27, 2016

Well, after manually merging your PR (due to me updating the version identifier) I can tell you that the only unit test that peepdf has at the moment, see also #2, failed with this PR. So I guess there's some work to be done :P

@Titotix
Copy link
Author

Titotix commented Oct 30, 2016

Yap,
I am pretty sure it is only because of this commit 25ae276
Perhaps that was not a good idea to erase this error code return but for now it is basically useless as it is a constant which is returned.

@Titotix
Copy link
Author

Titotix commented Nov 3, 2016

As discuss in jesparza#57, main changes compared to your work are :

  • new PDFOutput class which include methods like getPeepDict, getPeepXML and a new one GetPeepReport (to have a non formated report --> like the one we get when we launch the tool on cli). The code related to getPeepReport was duplicated in peepdf.py and PDFConsole.py, not any more here.
  • peepdf/constants.py to have all constants define like authors info but also error path file.
  • reintroduce PDFConsole method runit() (which you may just delete unintentionally)
  • introduce function to check hash against VT db in PDFCore.py

@Titotix
Copy link
Author

Titotix commented Nov 6, 2016

I revert my change and put back an error code "0" to PDFCore.parse()
Now the test pass successfully

Edit: Whoa, that wasn't supposed to increase the coverage ^^'

@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 21.778% when pulling 78764f6 on Titotix:merge-jb-tm into b4fa279 on jbremer:master.

@jbremer
Copy link
Member

jbremer commented Nov 7, 2016

Thanks. I'll first have to create one or two more unit tests to ensure that the usage, as we have it in @cuckoosandbox, remains working properly.

jbremer pushed a commit that referenced this pull request Dec 8, 2021
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.

3 participants