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

Refactoring of info/langs/mod.rs #948

Merged
merged 3 commits into from
Jan 28, 2023
Merged

Refactoring of info/langs/mod.rs #948

merged 3 commits into from
Jan 28, 2023

Conversation

o2sh
Copy link
Owner

@o2sh o2sh commented Jan 28, 2023

Folllowing #937, I couldn't help but notice that info/langs/mod.rs could need some refactoring for better readability but also for performance reasons.

Covered in this PR:

  • language::loc is called only in get_loc_by_language and reused in get_total_loc
  • Better naming for the methods and simplified signatures - up for discussion.
  • Extract some of the code logic from info/langs/mod.rs into their respective info fields: LanguageInfo and LocInfo

Copy link
Collaborator

@spenserblack spenserblack left a comment

Choose a reason for hiding this comment

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

Looks good!

}

pub fn get_language_statistics(
pub fn get_loc_by_language_sorted(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Better naming for the methods and simplified signatures - up for discussion.

I guess, on this subject, _loc_ could potentially be confusing to new contributors, since "loc" is frequently an abbreviation for "location". lines_by_language_sorted might be slightly more understandable to newcomers.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Good point, I added some documentation to clarify the terminology and explain the method's intent.

@o2sh o2sh merged commit fdf435d into main Jan 28, 2023
@o2sh o2sh deleted the chore/refacto-langs branch January 28, 2023 21:28
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.

None yet

2 participants