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.

Sign up for GitHub

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

Jump to bottom

fix(curriculum/python): improve shortest path algorithm project #53428

Merged
merged 9 commits into from
May 21, 2024
Merged

fix(curriculum/python): improve shortest path algorithm project #53428

merged 9 commits into from
May 21, 2024

Conversation

Dario-DC
Copy link
Contributor

@Dario-DC Dario-DC commented Jan 29, 2024 •
edited

Checklist:

Closes #53395

@Dario-DC Dario-DC added scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory. new python course labels Jan 29, 2024
@github-actions github-actions bot added the scope: i18n language translation/internationalization. Often combined with language type label label Jan 29, 2024
@ghost
Copy link

ghost commented Jan 29, 2024 •
edited by ghost

👀 Review this PR in a CodeSee Review Map

View the CodeSee Map of this change

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@ilenia-magoni ilenia-magoni mentioned this pull request Feb 6, 2024
Open
36 tasks
@camperbot

This comment was marked as resolved.

Sign in to view
3 similar comments
@camperbot

This comment was marked as resolved.

Sign in to view
@camperbot

This comment was marked as resolved.

Sign in to view
@camperbot

This comment was marked as resolved.

Sign in to view
@Dario-DC
fix description/hints and add step 13
3859821 
fix test and description

add img and new steps

fix: test step 13

add missing tests + various fixes

fix typos
@Dario-DC Dario-DC marked this pull request as ready for review May 7, 2024 08:36
@Dario-DC Dario-DC requested a review from a team as a code owner May 7, 2024 08:36
@Dario-DC Dario-DC added the status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. label May 7, 2024
ilenia-magoni
ilenia-magoni requested changes May 7, 2024
View reviewed changes
Copy link
Contributor

@ilenia-magoni ilenia-magoni left a comment

Choose a reason for hiding this comment

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

The title confuses me. What is algorithm design? I have not learn anything about that in this. I have learned about dictionaries, tuples and lists tho. Can't we change the name?

...hon/learn-algorithm-design-by-building-a-shortest-path-algorithm/6578b13757611e2825beb8a5.md Show resolved Hide resolved
@Dario-DC @ilenia-magoni
Apply suggestions from code review
aa648eb 
Co-authored-by: Ilenia <26656284+ilenia-magoni@users.noreply.github.com>
fhsinchy
fhsinchy approved these changes May 13, 2024
View reviewed changes
Copy link
Contributor

@fhsinchy fhsinchy left a comment

Choose a reason for hiding this comment

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

I didn't find any issues or inconsistencies 👍

@Dario-DC
Copy link
Contributor Author

The title confuses me. What is algorithm design? I have not learn anything about that in this. I have learned about dictionaries, tuples and lists tho. Can't we change the name?

The original name for this was "Learn Data Structures..." but the merge sort algorithm project has that name.
I'm okay with changing the name, but not sure about which name we should use.

@zairahira zairahira mentioned this pull request May 14, 2024
Closed
9 tasks
@Dario-DC
Copy link
Contributor Author

The title confuses me. What is algorithm design? I have not learn anything about that in this. I have learned about dictionaries, tuples and lists tho. Can't we change the name?

Can we please get this merged and then deal with the new title in a separate PR? It's becoming a pain.

@camperbot

This comment was marked as resolved.

Sign in to view
@camperbot

This comment was marked as resolved.

Sign in to view
@freeCodeCamp freeCodeCamp deleted a comment from camperbot May 20, 2024
@Dario-DC Dario-DC enabled auto-merge (squash) May 20, 2024 13:03
@moT01
Copy link
Member

moT01 commented May 20, 2024

Is this the way we typically do this? Don't we just make changes to English and then run an i18n sync after it gets merged?

@Dario-DC
Copy link
Contributor Author

Dario-DC commented May 20, 2024 •
edited

Is this the way we typically do this? Don't we just make changes to English and then run an i18n sync after it gets merged?

@moT01 To be honest, I'm following what Naomi told me to do (we already did it a couple of times) but I don't understand the point of doing it. If you have an answer, you're very welcome.

I can get rid of the changes to the i18n folders, but then tests will fail for Italian and Portuguese. Can we merge the PR then?

@moT01
Copy link
Member

moT01 commented May 20, 2024

No, if Naomi said to do it like this - we should do it like this. I wasn't sure, that's why I asked.

@Dario-DC Dario-DC merged commit 8d638df into freeCodeCamp:main May 21, 2024
21 of 22 checks passed
@Dario-DC Dario-DC deleted the fix/shortest-path-alg branch May 21, 2024 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fhsinchy fhsinchy fhsinchy approved these changes

@ilenia-magoni ilenia-magoni ilenia-magoni approved these changes

Assignees
No one assigned
Labels
new python course scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory. scope: i18n language translation/internationalization. Often combined with language type label status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Shortest Path Algorithm - Step 21 to 25 - Loop dictionaries not explained
5 participants
@Dario-DC @camperbot @moT01 @fhsinchy @ilenia-magoni