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

Claudine's final report #19

Merged
merged 11 commits into from
Dec 2, 2024
Merged

Claudine's final report #19

merged 11 commits into from
Dec 2, 2024

Conversation

mingness
Copy link
Contributor

@mingness mingness commented Oct 31, 2024

No description provided.

@mingness mingness marked this pull request as draft November 1, 2024 09:30
@mingness mingness marked this pull request as ready for review November 1, 2024 11:50
@mingness mingness changed the title Cchen final Claudine's final report Nov 1, 2024
@SableRaf
Copy link
Contributor

SableRaf commented Nov 1, 2024

Thanks for the final report Claudine! Tagging @Stefterv for a review before merging.

@SableRaf
Copy link
Contributor

SableRaf commented Nov 1, 2024

Quick feedback: I'd have liked to see more about any limitations and/or known issues with the work in its current state. Similarly, a Future Goals section would be welcome.

@mingness
Copy link
Contributor Author

mingness commented Nov 2, 2024

Sure, I interpreted this to be a report to consolidate all information, until I saw the reports of my fellow grantees, and I realized I might have tried to create the wrong thing. I could put this report somewhere else, and try to create a more lean and targeted report.

@mingness mingness marked this pull request as draft November 2, 2024 00:12
@mingness mingness marked this pull request as ready for review November 3, 2024 00:10
@mingness
Copy link
Contributor Author

mingness commented Nov 3, 2024

Ok, have completely refactored the report. I think I would have benefited from a report template. :)

@Stefterv
Copy link
Collaborator

Stefterv commented Nov 7, 2024

Sorry it took me a minute to review, looks good to me!

@SableRaf
Copy link
Contributor

SableRaf commented Nov 8, 2024

@mingness I added a comment above about the documentation site. Also clarified the README. Hope that helps!

@mingness
Copy link
Contributor Author

@SableRaf Is there something more missing from this PR that I need to do before it an be merged? Thanks.

@SableRaf
Copy link
Contributor

SableRaf commented Dec 1, 2024

@SableRaf Is there something more missing from this PR that I need to do before it an be merged? Thanks.

@mingness Glad you asked! 😄 My comment about the documentation website is still open

@mingness
Copy link
Contributor Author

mingness commented Dec 1, 2024

Ah, ok. I hadn't realized that was a request for something more. Apologies. I don't actually understand what the request is - You've just said in that comment that you "added a comment above about the documentation site" - what is the comment above? Can you remind me directly what is missing? Many thanks.

@mingness
Copy link
Contributor Author

mingness commented Dec 1, 2024

Perhaps it's possible you made a comment but didn't actually send it?

@SableRaf
Copy link
Contributor

SableRaf commented Dec 2, 2024

@mingness: My apologies! You're right. I didn't realize my review was never published.

@mingness
Copy link
Contributor Author

mingness commented Dec 2, 2024

@SableRaf no problem, it makes sense now. I've included your comment into the report. Please take a look 🙏 Thank you.

@SableRaf SableRaf merged commit fdd76ff into processing:main Dec 2, 2024
@SableRaf
Copy link
Contributor

SableRaf commented Dec 2, 2024

lgtm sorry again for the delay!

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.

3 participants