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

fix: to prevent spell checking errors when a user home directory has … #92

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

benjam-es
Copy link
Contributor

What:

  • Bug Fix

Description:

When running the test suite locally, my home directory is atum which is seen as a mis-spelling. As we usaully exclude that part of the directory elsewhere, this issue only arrises on the constant check within the Cache class.

Screenshot 2025-01-13 at 09 31 35

To prevent this, we ensure that the project path is inferred within the class to match other code, and we also ignore those words that are found within the project directory base path.

Screenshot 2025-01-13 at 12 32 01

Added in a similar way to presets

Related:

@benjam-es
Copy link
Contributor Author

@nunomaduro @c0nst4ntin #95 is an alternative approach to fixing the test suite locally without ignoring words, although I believe this approach is more useful as general users might have use DIR within constants in their projects

@c0nst4ntin c0nst4ntin added the duplicate This issue or pull request already exists label Jan 13, 2025
@benjam-es
Copy link
Contributor Author

@c0nst4ntin can this be the first issue and #95 be the duplicate? as I say - I think this has more benefit

@c0nst4ntin
Copy link
Collaborator

Sure. Both solutions can work. I will change the labels and leave the decision to Nuno

@c0nst4ntin c0nst4ntin added bug Something isn't working and removed duplicate This issue or pull request already exists labels Jan 13, 2025
…an unrecognise words, add that home directory words to the ignore array

Signed-off-by: Ben James <[email protected]>
@benjam-es benjam-es force-pushed the fix/skip-checking-home-directory branch from c91ec1b to 69a675b Compare January 18, 2025 15:38
@c0nst4ntin
Copy link
Collaborator

Hey @benjam-es , I took another look at this. Sorry for the delay. Generally, I like the idea, as the project name might sometimes also be very specific, so this would be ignored automatically as well.
 

  1. I am unsure if the use case you mentioned at the top is still required. 
    I believe Nuno stopped checking the constant values in 4f81fc4

  2. Maybe you could also write a test for the new functionality checking if the isWordIgnored() returns true for parts of the project path.

  3. I am a bit unsure about this, as it might introduce inconsistent behaviour on different machines, as the project folder might be called differently. For example, your username might be ignored because of your path. Then if you use it in a doc block, it would not fail. For other developers or on the CI, it would fail because the path is different.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants