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

Update derive_lang_from_path to derive from any part of path #222

Merged
merged 2 commits into from
Jan 18, 2025

Conversation

george-gca
Copy link
Collaborator

@george-gca george-gca commented Nov 14, 2024

🔤 Polyglot PR

Currently derive_lang_from_path only considers small parts of the path. This changes to consider any part in any/part/of/the/path.txt. This allows, for example, creating files like assets/data/en-us/cards.json and assets/data/pt-br/cards.json and setting its permalink to /assets/data/cards.json.

I need help setting tests for this, I have no idea where to start.

typing as crazy

Type of change

  • Docs update (changes to the readme or a site page, no code changes)
  • Ops wrangling (automation or test improvements)
  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Sweet release (needs a lot of work and effort)
  • Something else (explain please)

Checklists

  • If modifying code, at least one test has been added to the suite

@george-gca
Copy link
Collaborator Author

Now just missing the testing part.

@untra
Copy link
Owner

untra commented Jan 10, 2025

fwiw I've made progress writing rspec tests for derive_lang_from_path, and in the process I've lost faith doc.relative_path actually works at all. I haven't forgotten about this, but derive_lang_from_path needs even further adjustment.

I'm coordinating stories and work for a v1.9 release. Thanks again for your help @george-gca ! keep poking me, I really appreciate your contributions and support.

@george-gca
Copy link
Collaborator Author

Just out of curiosity, what are the advantages of rspec?

I made a fork of an official jekyll plugin, and I saw that they use minitest. Tbh I don't have enough knowledge of testing in ruby.

@untra
Copy link
Owner

untra commented Jan 11, 2025

Just out of curiosity, what are the advantages of rspec?

I made a fork of an official jekyll plugin, and I saw that they use minitest. Tbh I don't have enough knowledge of testing in ruby.

both testing specs are fairly similar and can accomplish the same things. I don't think one library is better than the other.
However I have already established a number of rspec tests and I don't want to consider a different ruby testing library unless someone gives me a good reason to.

check out PR #227 I opened implementing a few tests for derive_lang_from_path. I'm inclined to merge in this PR, and then mine to add the tests and give you credit for the edits.

@george-gca
Copy link
Collaborator Author

Thanks, it would be nice. Also, is it possible to add some kind of code coverage badge? Maybe it would be useful to have this, or not.

Copy link
Owner

@untra untra left a comment

Choose a reason for hiding this comment

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

approving and merging this, before adding in tests

@untra
Copy link
Owner

untra commented Jan 18, 2025

Thanks, it would be nice. Also, is it possible to add some kind of code coverage badge? Maybe it would be useful to have this, or not.

will get to this for the next release. I'm making an issue to followup on this.

@untra untra merged commit 4706a86 into untra:master Jan 18, 2025
1 check passed
@george-gca george-gca deleted the patch-1 branch January 18, 2025 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants