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

modified API to imageHash instead of hasImage #88

Merged
merged 3 commits into from
Sep 16, 2021
Merged

Conversation

chomosuke
Copy link
Owner

@chomosuke chomosuke commented Sep 15, 2021

I think there's only 3 places to change?

@chomosuke chomosuke added the documentation Improvements or additions to documentation label Sep 15, 2021
@chomosuke chomosuke added this to the Sprint 2 milestone Sep 15, 2021
@waltervan00
Copy link
Collaborator

Would you need to change Card.md as well?

@chomosuke chomosuke self-assigned this Sep 16, 2021
@chomosuke
Copy link
Owner Author

I believe I have done that? What change specifically do you refer to

@waltervan00
Copy link
Collaborator

Oh apologies.

Copy link
Collaborator

@shangzhel shangzhel left a comment

Choose a reason for hiding this comment

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

You have yet to update GET /api/image.

docs/api/Card.ts Outdated
@@ -9,7 +9,7 @@ export interface Card {
email: string;
jobTitle: string;
company: string;
hasImage: boolean;
imageHash?: string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use the Hashed type.

import { Hashed } from './Hashed';
Suggested change
imageHash?: string;
imageHash?: Hashed;

Copy link
Owner Author

@chomosuke chomosuke Sep 16, 2021

Choose a reason for hiding this comment

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

I thought Hashed specifically refers to password hash. Where as imageHash is just a fairly unique string associated with the image?

@chomosuke
Copy link
Owner Author

chomosuke commented Sep 16, 2021

You mean image.md?

image.md does not need to update because that endpoint actually does not get modified, except cache-control.

Copy link
Collaborator

@shangzhel shangzhel 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 to me.

@chomosuke chomosuke merged commit 9131383 into docs Sep 16, 2021
@chomosuke chomosuke deleted the richard/doc/imageHash branch September 16, 2021 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants