8000 remove glog dependency by jhawk28 · Pull Request #293 · hypermodeinc/ristretto · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

remove glog dependency #293

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 5 commits into from
Closed

remove glog dependency #293

wants to merge 5 commits into from

Conversation

jhawk28
Copy link
Contributor
@jhawk28 jhawk28 commented Jan 27, 2022 •
edited by manishrjain
Loading

updated go mod deps

glog adds global flags so it is not a good library (adding to an existing program causes a panic if a flag name conflicts)

This change is Reviewable

@jhawk28 jhawk28 requested a review from manishrjain as a code owner January 27, 2022 19:33
@CLAassistant
Copy link
CLAassistant commented Jan 27, 2022 •
edited
Loading

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 4 committers have signed the CLA.

✅ hawkingrei
✅ jhawk28
❌ chux0519
❌ manishrjain
You have signed the CLA already but the status is still pending? Let us recheck it.

@jhawk28
Copy link
Contributor Author
jhawk28 commented Jan 27, 2022

lol, just saw: #292

@marcind marcind mentioned this pull request Feb 1, 2022
Closed
@franchb franchb mentioned this pull request Feb 3, 2022
Merged
@Emyrk
Copy link
Emyrk commented Mar 22, 2022

Can we please merge this. Glog spins off a go routine that cannot be closed on it's init() function, breaking unit tests that check for go routine leaks.

@NamanJain8

@michalb-goflink michalb-goflink mentioned this pull request Aug 26, 2022
Closed
@mkevac mkevac mentioned this pull request Apr 5, 2023
Open
@David-Mao
Copy link

Can we please merge this. Glog spins off a go routine that cannot be closed on it's init() function, breaking unit tests that check for go routine leaks.

Ditto. @mangalaman93

@mholt
Copy link
mholt commented Aug 26, 2023

Glog is not intended for use outside Google:

The repository contains an open source version of the log package used inside Google. The master copy of the source lives inside Google, not here. The code in this repo is for export only and is not itself under development. Feature requests will be ignored.

Why is this PR still unmerged??

@manishrjain
Copy link
Contributor

@mholt , I maintain a fork of ristretto. I have this and other fixes, if you want to use that. I know Datadog uses my fork.

outcaste-io/ristretto@bd658a4

@mholt
Copy link
mholt commented Aug 28, 2023

@manishrjain Thanks; since this is a dependency of a dependency of a ... (x4) ... I suppose I will use a replace in my go.mod -- that might do...

@mangalaman93 mangalaman93 changed the base branch from master to main August 29, 2023 13:43
@mangalaman93
Copy link
Member

@mholt will it be possible for you rebase it on latest main instead of mater. I already changed the base branch here to main. Apologies for the delay. Thanks

@mholt
Copy link
mholt commented Aug 29, 2023

@mangalaman93 Thanks; this isn't my PR though, @jhawk28 would have to do that.

manishrjain and others added 5 commits August 29, 2023 10:40
@manishrjain @jhawk28
feat(SuperFlag): Allow a way to get error while merging
6ec1197
@manishrjain @jhawk28
feat(Ristretto): Introduce SetIfPresent and refactor codebase (#282)
811f221 
* Introduce SetIfPresent and move metrics out to new file.
* Introduce ShouldUpdate, to allow Dgraph to only update the cache with a higher version entry.
* Remove interfaces.
* Rename structs for clarity.
@chux0519 @jhawk28
feat(mmap): support msync for windows (#283)
28886d3 
See: https://docs.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-flushviewoffile
@hawkingrei @jhawk28
*: fix data race in the cache (#294)
0667ea9 
* *: fix data race in the cache

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

* fix
@jhawk28
remove glog since glog adds global flags
6b6921e 
update go mod deps
@jhawk28
Copy link
Contributor Author
jhawk28 commented Aug 29, 2023

I did a rebase but it didn't appear to work correctly. I would recommend just creating another PR.

@jhawk28 jhawk28 closed this Aug 29, 2023
@jhawk28 jhawk28 deleted the glogless branch August 29, 2023 14:51
@jhawk28
Copy link
Contributor Author
jhawk28 commented Aug 30, 2023

submitted a new pr #350

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@manishrjain manishrjain Awaiting requested review from manishrjain

@akon-dey akon-dey Awaiting requested review from akon-dey

@joshua-goldstein joshua-goldstein Awaiting requested review from joshua-goldstein

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@jhawk28 @CLAassistant @Emyrk @David-Mao @mholt @manishrjain @mangalaman93 @chux0519 @hawkingrei
0