-
-
Notifications
You must be signed in to change notification settings - Fork 338
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
Release notes v0.1.6 #3506
Release notes v0.1.6 #3506
Conversation
I forgot to mention this in our last team meeting, do we need to add another page in our docs for v0.1.6 upgrade instructions if they are the same as v0.1.5? |
They're not the same as v0.1.5. We would need a separate page. There are changes in steps involved for Install from scratch. It's tracked in this issue: #3505 |
We should also add a Basecamp task to 0.1.7 and the template to make sure we have upgrade instructions for the release, @Anish9901 can you do this? |
Some thoughts based on a quick skim, I'll review in more detail next week once these are addressed:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Format-wise, it's fine.
The only blocking changes I request are to
- sort out the links for removing the NodeJS dependency when building from source, and
- Fix the incorrect grammar where I noted it in a specific comment.
Generally, I think it's worth the effort to try to make the descriptions of PRs short and dense enough that they don't need to wrap except in edge cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, this is good to go from my end. Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made some changes to the summary, the rest looks okay to me, but I haven't done a line-by-line review of each of the changes listed.
I will leave it to @seancolsen to do the line-by-line review since he wrote the previous release notes, and judging by mathesar-foundation/mathesar-wiki@7883614 he has some opinions about this.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a number of small points of critique. These are mostly detail-level things. I don't want to hold up the release for the sake of fixing these necessarily. We could potentially fix them afterwards.
As much as this review is a critique of your work in writing the release notes, it's also a critique of the release notes process, i.e. the Writing Release Notes wiki page that I created. I'd like to find ways to improve (or better document) this process where we can. For that reason, I'm dwelling on this PR review a bit more than I normally would. Hypothetically, I could quickly push a commit to this PR implementing my suggested changes and call it a day. But I want to get our process honed so that it's easy to hand off. Towards that end, today I added a section to that wiki page which lists guidelines to follow when writing the release notes. I'll admit that even I didn't even follow all these guidelines when authoring the 0.1.5 release notes — which is one reason I think it'll be helpful to keep in the document as a sort of checklist. For the sake of speed (and because the stakes here are quite low), I'll assume that nobody on the team wishes to contest any of these new release notes guidelines, but I'm happy to discuss as needed.
My critique:
-
I'm seeing signs that perhaps you didn't follow the first step in the process. Did you run
./find_missing_prs.sh 0.1.6
?-
If you had run that command, then I would expect to see double-quoted strings after all PR URLs, as shown in the 0.1.5 release notes. This is special markdown which generates tool tips like this:
Those tool tips are there to make it a little bit easier for reader to see the original PR titles.
-
Also, there appear to be some PRs missing from the release notes: Merge pull request #3460 from mathesar-foundation/0.1.5 #3463 Add Debug image docs #3513 Remove 3.12 support from docs #3503 Merge pull request #3468 from mathesar-foundation/upgrade_instruction_fix #3469 Merge pull request #3475 from mathesar-foundation/version_number_in_docs #3476. I see those PRs when I run the script in this PR's branch.
If you did run the script, then I'd like to troubleshoot why it's not giving the behavior I would expect. If you didn't run the script, I'd like to understand why. Do you have an opinion against this script-based approach? Or could we improve our release notes process documentation to make this step more obvious?
-
-
I suggest modifying the order of items within the Improvements section as follows:
- i18n
- auto-expanding text fields
- Python versions
- NodeJS
This order focuses on user-facing changes first and better showcases our recent work on i18n, which I think readers are likely to find interesting and impressive.
-
Adjust the title of the i18n item to improve clarity and become grammatically parallel with the other list items:
You can now configure Mathesar's UI to display in Japanese
-
Add descriptions for all four items in the improvements section. For example, here's a proposed description for the i18 improvement:
The language setting is stored per-user and can be modified when logging in or when editing a user. This changes the text displayed on buttons and other UI elements within Mathesar. It does not change the display of data within your database (e.g. table names, column names, and cell values). We are hoping to support more languages beyond English and Japanese eventually. Please reach out to us if your are interested in helping to add more translations!
-
Add screenshots for the "auto-expanding text fields" and "i18n" features. I sent a screenshot here for the i18n feature that you can use.
-
Re-categorize mid-cycle regressions.
- Remove entries for Fix regression with record selector not filtering #3488 and Prevent record selector inputs from growing taller #3495. Instead, list these within the "auto-expanding text fields" improvement.
- Remove the entry for Fix layout problem in Data Explorer actions pane #3501. Instead, list it under the "i18n" improvement.
See my (newly written) guideline for more rationale about these changes.
I think your guidelines make sense, @seancolsen. Since there's not much left to do in the release process, I think it's fine to make these changes before release, unless @Anish9901 thinks they will take more than a day. |
I wasn't able to get the script running on a Mac @seancolsen, That's why I decided not to use it, I wasn't aware that it added a bunch of metadata about linked issues. I've got it to work using a linux machine now. Here's the specific error that I was facing on a Mac:
|
@Anish9901 it looks like you found some portability issues with my bash script. Wonderful! Instead of ignoring this script, I'd like to fix it. The hover text on PR hyperlinks is cool, but the main value proposition of the script is that it makes it really fast to generate the release notes. I've opened #3515 to track fixes to the script. I don't imagine this being too hard, so I think we could take it up for the next release. In the mean time, it's great that you've found a work-around using Linux. I assume you can use this workflow to push changes that address my critique. Let me know if you need any help. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Splendid! @Anish9901 you've addressed all of my feedback perfectly.
I merged 0.1.6
into this branch which I think should fix the CI errors. (Mkdocs was complaining when trying to build in strict mode because the upgrade instructions page was a dead link since that page got implemented separately as part of #3507.) Assuming those CI errors resolve, then this is ready to merge.
Release notes for v0.1.6
Checklist
Update index.md
).develop
branch of the repositoryvisible errors.
Developer Certificate of Origin
Developer Certificate of Origin