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 base64 function names #26

Merged
merged 4 commits into from
Jun 17, 2024
Merged

Update base64 function names #26

merged 4 commits into from
Jun 17, 2024

Conversation

firasm
Copy link
Contributor

@firasm firasm commented Aug 17, 2023

I changed the names of these two functions to make them simpler to use/remember (and so the type isn't part of the function name)

I'll do a find and replace in instructor_datascience_bank to fix this on any questions that are currently using this.

I changed the names of these two functions to make them simpler to use (and so the type isn't part of the function name)

I'll do a find and replace in instructor_datascience_bank to fix this on any questions that are currently using this.
@firasm firasm requested a review from Bluesy1 August 17, 2023 21:01
Copy link
Collaborator

@Bluesy1 Bluesy1 left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me, and looks correct - I'll have to update open-resources/instructor_datascience_bank#232 once this is merged and released so I'll hold off requesting any reviews on that until then.

Should probably make some tests for these functions at somepoint to actually make sure they stay working, but thats out of scope for this PR

Copy link
Collaborator

@Bluesy1 Bluesy1 left a comment

Choose a reason for hiding this comment

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

Pushed a commit to fix what was broken, and fixed the merge conflict, if you still want to make this change

@Bluesy1 Bluesy1 closed this in #36 Jun 17, 2024
@Bluesy1 Bluesy1 merged commit 9b55dad into main Jun 17, 2024
1 check passed
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.

2 participants