luckyframework / luckyframework/avram
Stop using Symbols
Nobody has claimed this yet.
- Dominant language
- Crystal
- Stars
- 183
- Forks
- 67
- PR merge metrics
- No merged PRs in 30d
Description
Related: https://github.com/luckyframework/avram/issues/1061
While working on https://github.com/luckyframework/avram/pull/1059 I'm having to do a lot of transforming since columns will need to be wrapped in quotes. Since we're using `Symbol` all over the place, I have to make sure I convert in to `String`, and then it ends up getting used in a string anyway... I thought we had an issue already talking about this, but I'm not sure where it's at.
On the one hand, this looks nice `add_index :users, [:email]`, but it's not much more than `add_index "users", ["email"]`. The other issue is with a `Symbol` is we lose the ability to do checks like `.ends_with?` without first converting to a String anyway.
I know in some cases we allow both `Symbol` and `String`, but why not just enforce a single type for consistency and reduce a few Unions in the code.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reading related issue #1061 and the changes discussed in pull request #1059, then trace the Symbol/String conversions they expose. Done means deciding and applying one consistent column-name type across the affected database APIs, while preserving the supported usage described here and reducing unnecessary unions.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- crystal
- Domain
- databases
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 30/100