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

Feature/hash so #3715

Closed
wants to merge 2 commits into from
Closed

Conversation

AlonZivony
Copy link
Collaborator

1. Explain what the PR does

9fb3713 feat(events): add hash to symbols_loaded event
b1e2141 feat(events): add hash to shared_object_loaded event

Related to #3576

2. Explain how to test it

3. Other comments

@rafaeldtinoco
Copy link
Contributor

rafaeldtinoco commented Nov 29, 2023

@AlonZivony LGTM, I like it. YOu need to (re)generate the man pages, like the error is telling you to. After that, since all tests passed I can merge right away.

There is also this test:

image

needs checking I believe.

Add file hash of the shared object loaded to the event.
The behavior matches the one from the sched_process_exec event.
Add file hash of the shared object loaded to the event.
@rafaeldtinoco
Copy link
Contributor

image

You need to update the man pages (make man).

@AlonZivony
Copy link
Collaborator Author

So it is weird, because I have ran make man.
You can see that there are .1 pages in the first commit. If I run it again there is no change.

@geyslan
Copy link
Member

geyslan commented Nov 30, 2023

@AlonZivony it's odd. 🤔

I noticed that other .1 files were changed by a lower version of pandoc but the required one.

@geyslan
Copy link
Member

geyslan commented Nov 30, 2023

@AlonZivony it's odd. 🤔

I noticed that other .1 files were changed by a lower version of pandoc but the required one.

Well, this made the required changes: #3720

The comment in pr.yaml was misleading and the docker image was not using a pinned pandoc version; but now it's back on track.

@rafaeldtinoco
Copy link
Contributor

The comment in pr.yaml was misleading and the docker image was not using a pinned pandoc version; but now it's back on track.

With this said, I believe @AlonZivony should rebase, push and then we can merge after tests succeed.

@rafaeldtinoco
Copy link
Contributor

Merged by #3721

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

Successfully merging this pull request may close these issues.

None yet

3 participants