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

reorganized the code layout #74

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

ibrahimgb
Copy link

Description

i have reorganized the code layout for the data file I have not edited nor changed the data

Fixes # (issue)

Type of change

reorganized

Please delete options that are not relevant.

  • Updated the dataset

Checklist:

  • I checked that there's no dataset update (can be done by running npm run update-dataset)
  • npm test passes on my machine
  • npm run lint passes on my machine

@ZibanPirate
Copy link
Member

hey @ibrahimgb thanks for the PR, is there a need or a reason for this change?

@ZibanPirate ZibanPirate added the enhancement New feature or request label Feb 13, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #74 (cf2c0e6) into develop (3fe8e06) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           develop       #74   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           20        20           
  Lines          173       173           
  Branches        33        33           
=========================================
  Hits           173       173           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3fe8e06...cf2c0e6. Read the comment docs.

@ibrahimgb
Copy link
Author

reason for this change: the hole data was crammed in one line. I just organize it to make it easy for future contributions (who want to update the dataset).

hey @ibrahimgb thanks for the PR, is there a need or a reason for this change?

@Fcmam5
Copy link
Collaborator

Fcmam5 commented Feb 27, 2022

Hi @ibrahimgb the idea was to pull the dataset from the files in https://github.com/Fcmam5/algeria-api which is now super outdated.
And the data in one-line here is meant to use a compressed version of the dataset for a tiny-ish performance gain.

Or, do you have any suggestions for doing this better? We can keep the prettified version of the dataset and only minified when building the library

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Addressing Feedback
Development

Successfully merging this pull request may close these issues.

4 participants