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

refactor: remove redundant references #25571

Merged

Conversation

hamirmahal
Copy link
Contributor

Problem

There are some unnecessary &s in the codebase. We take references in places we don't need to.

Changes

There are no frontend changes.

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

Does this work well for both Cloud and self-hosted?

It doesn't have an impact.

How did you test this code?

cargo test passes locally.

@posthog-bot
Copy link
Contributor

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week. If you want to permanentely keep it open, use the waiting label.

Copy link
Contributor Author

@hamirmahal hamirmahal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Twixes are you open to this pull request?

@posthog-bot posthog-bot removed the stale label Oct 23, 2024
@Twixes Twixes requested review from aspicer and removed request for aspicer October 24, 2024 15:44
Twixes
Twixes previously requested changes Oct 24, 2024
Copy link
Member

@Twixes Twixes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aspicer is the one with context on our Rust implementation of funnels, but currently this PR is somewhat unreviewable, as most of the changed lines are just formatting by cargo fmt, rather than actual code changes. Please revert the formatting changes, so that the PR is targeted to just references

@hamirmahal hamirmahal force-pushed the refactor/remove-redundant-references branch from 91a736b to fd7e5ab Compare October 24, 2024 20:11
@hamirmahal hamirmahal requested a review from Twixes October 24, 2024 20:11
@Twixes Twixes requested review from aspicer and removed request for Twixes October 28, 2024 09:35
@Twixes Twixes dismissed their stale review October 28, 2024 09:35

Outdated

@posthog-bot
Copy link
Contributor

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week. If you want to permanentely keep it open, use the waiting label.

@posthog-bot posthog-bot added stale and removed stale labels Nov 5, 2024
@posthog-bot
Copy link
Contributor

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week. If you want to permanentely keep it open, use the waiting label.

@posthog-bot posthog-bot added stale and removed stale labels Nov 13, 2024
Copy link
Contributor

@aspicer aspicer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@aspicer aspicer enabled auto-merge (squash) November 15, 2024 18:45
@aspicer aspicer merged commit 39659d7 into PostHog:master Nov 15, 2024
88 checks passed
@hamirmahal hamirmahal deleted the refactor/remove-redundant-references branch November 15, 2024 19:20
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