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

Adding merge_event function #202

Closed
wants to merge 3 commits into from
Closed

Adding merge_event function #202

wants to merge 3 commits into from

Conversation

ater49
Copy link
Contributor

@ater49 ater49 commented Mar 6, 2018

Adding merge_event in order to merge two similar events in one.

@codecov
Copy link

codecov bot commented Mar 6, 2018

Codecov Report

Merging #202 into master will decrease coverage by 0.12%.
The diff coverage is 12.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #202      +/-   ##
==========================================
- Coverage   64.28%   64.16%   -0.13%     
==========================================
  Files          23       23              
  Lines        3408     3416       +8     
==========================================
+ Hits         2191     2192       +1     
- Misses       1217     1224       +7
Impacted Files Coverage Δ
pymisp/api.py 47.06% <12.5%> (-0.25%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b50fc4...bf8d936. Read the comment docs.

@Rafiot
Copy link
Member

Rafiot commented Mar 6, 2018

This new method will be very confusing:

  • It doesn't copy the objects
  • What is supposed to happen if we have duplicate attributes/objects?

iirc, there is a merge event functionality in MISP. I think it would be better to open up this one to the API, so we stay consistent. @iglocska what do you think?

@ater49
Copy link
Contributor Author

ater49 commented Mar 6, 2018

Didn't see that function. I can close this PR and work to implement the existing core functionality.

@Rafiot
Copy link
Member

Rafiot commented Apr 12, 2018

We have something much better than merging events in the 2.4.90 release: MISP/MISP@caf53e0

It let's you extend an event with an other event without modifying the original one. It is especially useful then said event isn't yours.

Does it fits your needs?

@ater49
Copy link
Contributor Author

ater49 commented Aug 8, 2018

It fits!

I close this PR

@ater49 ater49 closed this Aug 8, 2018
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.

None yet

2 participants