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

5194 remove diesel dls alias #6057

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

GeronimoJohn
Copy link
Contributor

@GeronimoJohn GeronimoJohn commented Jan 14, 2025

Fixes #5194

👩🏻‍💻 What does this PR do?

This removes redundant aliasing of Diesel DSLs. According to Diesel's documentation, the dsl alias is only useful when dealing with a single table. We should use DSL for convenience if we are working with just one table in a module, but for the rest, we don't need to alias them.

💌 Any notes for the reviewer?

This is quite the hefty review. In order to make things easier, I've pushed the changes in batches. I was considering different PRs, but I didn't want our project to be filled with them.

See the link below for each commit. Hope it helps 😃

  1. Initial Batch
  2. 2nd Batch
  3. 3rd Batch
  4. 4th Batch
  5. Last Batch

🧪 Testing

📃 Documentation

  • Part of an epic: documentation will be completed for the feature as a whole
  • No documentation required: no user facing changes or a bug fix which isn't a change in behaviour
  • These areas should be updated or checked:
    1.
    6.

@GeronimoJohn GeronimoJohn added this to the v2.6.0 milestone Jan 14, 2025
@github-actions github-actions bot added back-end Back-end related Priority: Should have Team Korimako Laché, Aimee, Noel John, and Zachariah labels Jan 14, 2025
@roxy-dao roxy-dao self-assigned this Jan 15, 2025
Copy link
Contributor

@roxy-dao roxy-dao left a comment

Choose a reason for hiding this comment

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

Nice! Massive effort ~ Just a nitpicky comment, but I would have alias some of it to have the same name as the _dsl without the _dsl XD If that makes sense...

Just did a quick search and it seems there are still 551 more results for _dsl.
Screenshot 2025-01-16 at 8 43 45 AM

Could you keep the issue opened and do this in another PR since this one is already massive~?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
back-end Back-end related Priority: Should have Team Korimako Laché, Aimee, Noel John, and Zachariah
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor to remove redundant aliasing of diesel DSLs
2 participants