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

v6.0.0-alpha.0 #6254

Merged
merged 15 commits into from
Sep 23, 2022
Merged

v6.0.0-alpha.0 #6254

merged 15 commits into from
Sep 23, 2022

Conversation

flaviendelangle
Copy link
Member

@flaviendelangle flaviendelangle commented Sep 22, 2022

I removed from the changelog the PRs already listed in #6252, even when cherry picked.
I can put them in the changelog of each version if you want.

I will add the changelog from #6252 to this branch before merging to always have all the changelogs on next.

## 6.0.0-alpha.0

_Sep 22, 2022_

Copy link
Member Author

Choose a reason for hiding this comment

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

We are missing a small introduction about the alpha, if you have some ideas @joserodolfofreitas

Copy link
Member

@joserodolfofreitas joserodolfofreitas Sep 23, 2022

Choose a reason for hiding this comment

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

Sorry I was too late to answer this.
But we can give a good introduction when the the blog post is out:
https://deploy-preview-34424--material-ui.netlify.app/blog/mui-x-v6-alpha-zero/

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll do the release without it then and we will link the blogpost in the changelog when ready 👍

@flaviendelangle flaviendelangle mentioned this pull request Sep 22, 2022
@mui-bot
Copy link

mui-bot commented Sep 22, 2022

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 485 912.7 562.7 626.96 155.71
Sort 100k rows ms 556.3 1,068.1 821 831.3 163.305
Select 100k rows ms 150.7 291.8 203 215.8 47.153
Deselect 100k rows ms 117.9 207.3 193.1 172.76 33.674

Generated by 🚫 dangerJS against b02a4da

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@cherniavskii
Copy link
Member

@flaviendelangle

I removed from the changelog the PRs already listed in #6252, even when cherry picked.
I can put them in the changelog of each version if you want.

I will add the changelog from #6252 to this branch before merging to always have all the changelogs on next.

Shouldn't we just list all the changes compared to the previous release for each branch?
We might have changes released only in v5, or only in v6.

flaviendelangle and others added 4 commits September 22, 2022 14:32
Co-authored-by: Matheus Wichman <matheushw@outlook.com>
Signed-off-by: Flavien DELANGLE <flaviendelangle@gmail.com>
Co-authored-by: Matheus Wichman <matheushw@outlook.com>
Signed-off-by: Flavien DELANGLE <flaviendelangle@gmail.com>
Co-authored-by: Matheus Wichman <matheushw@outlook.com>
Signed-off-by: Flavien DELANGLE <flaviendelangle@gmail.com>
Co-authored-by: Matheus Wichman <matheushw@outlook.com>
Signed-off-by: Flavien DELANGLE <flaviendelangle@gmail.com>
@flaviendelangle
Copy link
Member Author

flaviendelangle commented Sep 22, 2022

@cherniavskii

We might have changes released only in v5, or only in v6.

Sure, but I don't see the problem for those PRs.
With my approach,
Something released only in v5 would only be on the v5 patch changelog
Something released only in v6 would only be on the v6 patch changelog

Shouldn't we just list all the changes compared to the previous release for each branch?

For me the debate is more for something that is released for both v5 and v6 (most of the time developed on one and ported on the other).
We can do several things:

  1. Only put the development PR in the v5 changelog (even when developing on next and cherry-picking on master, we would put the development PR to make sure that the person doing the development is tagged) => current version in this PR

  2. Put the development PR in both changelogs. There will be duplicates on the v6 CHANGELOG.md since we are copying the releases from master on it to have the full list. Should we put the highlights in both as well ?

  3. Put the development PR in whatever branch got the development and the cherry-picking PR on whatever branch got the cherry-picking. That's the most rigorous solution in term of content of the release. But that would be weird to tag me for a cherry-picking I did of a locale file written by a community member. And we have duplicates as well.

My least favorite is 3 because I really don't like to "steal" the community work on the changelog.
But between 1 and 2, I don't have a strong opinion.
My reasoning was that with 1, everything is on the CHANGELOG.md of the next branch anyway.

@cherniavskii
Copy link
Member

Let's take #6009 for example. It was merged into next branch and then backported to master in a separate PR #6251 and let's pretend backporting PR was opened and merged by someone else.

Given the options you proposed, in the changelog we will see:

  1. Only put the development PR in the v5 changelog (even when developing on next and cherry-picking on master, we would put the development PR to make sure that the person doing the development is tagged) => current version in this PR

    ## v6.0.0-alpha.0
    
    - 
    
    ## v5.17.4
    
    - [DataGrid] Do not publish `cellFocusOut` event if the row was removed (#6009) @cherniavskii 
    - 

    v6 release notes clearly miss a change. As a developer using v6, I won't see complete release notes and I wouldn't assume that everything from v5 is also included in v6 release, so it would look like this change is not included in v6

  2. Put the development PR in both changelogs. There will be duplicates on the v6 CHANGELOG.md since we are copying the releases from master on it to have the full list. Should we put the highlights in both as well ?

    ## v6.0.0-alpha.0
    
    - [DataGrid] Do not publish `cellFocusOut` event if the row was removed (#6009) @cherniavskii
    - 
    
    ## v5.17.4
    
    - [DataGrid] Do not publish `cellFocusOut` event if the row was removed (#6009) @cherniavskii
    - 

    v6 and v5 changelogs are self-sufficient which is good, but when doing the release we will have to go through each entry in the changelog and make sure to replace backported PRs with original PRs. This requires more work and can be error-prone.

  3. Put the development PR in whatever branch got the development and the cherry-picking PR on whatever branch got the cherry-picking. That's the most rigorous solution in term of content of the release. But that would be weird to tag me for a cherry-picking I did of a locale file written by a community member. And we have duplicates as well.

    ## v6.0.0-alpha.0
    
    - [DataGrid] Do not publish `cellFocusOut` event if the row was removed (#6009) @cherniavskii
    - 
    
    ## v5.17.4
    
    - [DataGrid] Do not publish `cellFocusOut` event if the row was removed (#6251) @flaviendelangle 
    - 

    v6 and v5 changelogs are self-sufficient. Auto-generated release notes would just work.

    But that would be weird to tag me for a cherry-picking I did of a locale file written by a community member

    I agree it's a bit weird, but:

    • technically, you are the author of the cherry-picking PR, although changes are not yours :)
    • original PR and its author will also be included in the release notes of corresponding library version
    • in the cherry-picking PR there will be link to the original PR

    And we have duplicates as well

    I would say it's an upside rather than a downside

Personally, I would rather prefer option (3):

  • v5 and v6 release notes are independent and self-sufficient
  • it's less work to create release notes (no need to replace cherry-picking PRs with original PRs)

@flaviendelangle
Copy link
Member Author

technically, you are the author of the cherry-picking PR, although changes are not yours :)

And about the highlight ?
Because that's the point that bother me the most.
Most community PRs are l10n related and a good part of them ends up in the highlight.
And that's where I would find very disturbing to put me as the author of the change.

@flaviendelangle
Copy link
Member Author

To avoid blocking the release, I went for option 2, even if it's a little more work.

@cherniavskii
Copy link
Member

@flaviendelangle

And about the highlight ?
Because that's the point that bother me the most.
Most community PRs are l10n related and a good part of them ends up in the highlight.
And that's where I would find very disturbing to put me as the author of the change.

Totally valid point!
I thought we can solve this with listing all co-authors of the merge commit, but GitHub API doesn't make it easy to do so.
Here's what info we have for the merge commit of #6230:

{
  "sha": "0e1eb7b200a19256ebfcd72e81de299a0ca10b13",
  "node_id": "C_kwDOD4LzcdoAKDBlMWViN2IyMDBhMTkyNTZlYmZjZDcyZTgxZGUyOTlhMGNhMTBiMTM",
  "commit": {
    "author": {
      "name": "Flavien DELANGLE",
      "email": "flaviendelangle@gmail.com",
      "date": "2022-09-21T06:37:47Z"
    },
    "committer": {
      "name": "GitHub",
      "email": "noreply@github.com",
      "date": "2022-09-21T06:37:47Z"
    },
    "message": "[I10n] Add Finnish (fi-FI) locale to Datepicker (#6219) (#6230)\n\nCo-authored-by: Petro Silenius <petro.silenius@gmail.com>",
    "tree": {
      "sha": "926891180732fc5b5127dd1e409b63d80d352e93",
      "url": "https://api.github.com/repos/mui/mui-x/git/trees/926891180732fc5b5127dd1e409b63d80d352e93"
    },
    "url": "https://api.github.com/repos/mui/mui-x/git/commits/0e1eb7b200a19256ebfcd72e81de299a0ca10b13",
    "comment_count": 0,
    "verification": {
      "verified": true,
      "reason": "valid",
      "signature": "-----BEGIN PGP SIGNATURE-----\n\nwsBcBAABCAAQBQJjKrE7CRBK7hj4Ov3rIwAAH/UIADXF137ZnXL24nqNoF4DLC8U\nxVMKuc8FT4cDFujK+qPvpUd5odP805Hetd5pzOTAJlgtP7AtAK/s8F28VP0aqp0A\nLCfLHqDJ2NoGD6lTFNZq/W8K4IJn6HpwOKZor//VZSX1fxiirQqtjObTdBczN2a7\nrX4y8kD6PvFecXSJ2YXMceD25ZehN5xis0Sng09TCcU8r0wbZuTuay2bimmVRikO\nIjNI1PxQ6pAF7XUoY6RcSd9P16evfAEu0/K8L0GTDEzC3QLquyluKYvmukqc3epV\n2yivam0Q/0b2TI4fHllISHR6131UPplHtlTnn/sd29tMpIAqNgRlIKlpwm6ejMs=\n=uPD0\n-----END PGP SIGNATURE-----\n",
      "payload": "tree 926891180732fc5b5127dd1e409b63d80d352e93\nparent de5740d801073519b49f2fa3600ed5f0d4dfc7a4\nauthor Flavien DELANGLE <flaviendelangle@gmail.com> 1663742267 +0200\ncommitter GitHub <noreply@github.com> 1663742267 +0200\n\n[I10n] Add Finnish (fi-FI) locale to Datepicker (#6219) (#6230)\n\nCo-authored-by: Petro Silenius <petro.silenius@gmail.com>"
    }
  },
  "url": "https://api.github.com/repos/mui/mui-x/commits/0e1eb7b200a19256ebfcd72e81de299a0ca10b13",
  "html_url": "https://github.com/mui/mui-x/commit/0e1eb7b200a19256ebfcd72e81de299a0ca10b13",
  "comments_url": "https://api.github.com/repos/mui/mui-x/commits/0e1eb7b200a19256ebfcd72e81de299a0ca10b13/comments",
  "author": {
    "login": "flaviendelangle",
    "id": 3309670,
    "node_id": "MDQ6VXNlcjMzMDk2NzA=",
    "avatar_url": "https://avatars.githubusercontent.com/u/3309670?v=4",
    "gravatar_id": "",
    "url": "https://api.github.com/users/flaviendelangle",
    "html_url": "https://github.com/flaviendelangle",
    "followers_url": "https://api.github.com/users/flaviendelangle/followers",
    "following_url": "https://api.github.com/users/flaviendelangle/following{/other_user}",
    "gists_url": "https://api.github.com/users/flaviendelangle/gists{/gist_id}",
    "starred_url": "https://api.github.com/users/flaviendelangle/starred{/owner}{/repo}",
    "subscriptions_url": "https://api.github.com/users/flaviendelangle/subscriptions",
    "organizations_url": "https://api.github.com/users/flaviendelangle/orgs",
    "repos_url": "https://api.github.com/users/flaviendelangle/repos",
    "events_url": "https://api.github.com/users/flaviendelangle/events{/privacy}",
    "received_events_url": "https://api.github.com/users/flaviendelangle/received_events",
    "type": "User",
    "site_admin": false
  },
  "committer": {
    "login": "web-flow",
    "id": 19864447,
    "node_id": "MDQ6VXNlcjE5ODY0NDQ3",
    "avatar_url": "https://avatars.githubusercontent.com/u/19864447?v=4",
    "gravatar_id": "",
    "url": "https://api.github.com/users/web-flow",
    "html_url": "https://github.com/web-flow",
    "followers_url": "https://api.github.com/users/web-flow/followers",
    "following_url": "https://api.github.com/users/web-flow/following{/other_user}",
    "gists_url": "https://api.github.com/users/web-flow/gists{/gist_id}",
    "starred_url": "https://api.github.com/users/web-flow/starred{/owner}{/repo}",
    "subscriptions_url": "https://api.github.com/users/web-flow/subscriptions",
    "organizations_url": "https://api.github.com/users/web-flow/orgs",
    "repos_url": "https://api.github.com/users/web-flow/repos",
    "events_url": "https://api.github.com/users/web-flow/events{/privacy}",
    "received_events_url": "https://api.github.com/users/web-flow/received_events",
    "type": "User",
    "site_admin": false
  },
  "parents": [
    {
      "sha": "de5740d801073519b49f2fa3600ed5f0d4dfc7a4",
      "url": "https://api.github.com/repos/mui/mui-x/commits/de5740d801073519b49f2fa3600ed5f0d4dfc7a4",
      "html_url": "https://github.com/mui/mui-x/commit/de5740d801073519b49f2fa3600ed5f0d4dfc7a4"
    }
  ]
}

In the commit message there are co-authors mentioned ("Co-authored-by: Petro Silenius", but we will have to parse commit message to get them and then somehow get a GitHub username for each co-author.

I think at this point option (2) is indeed the most optimal.

@flaviendelangle flaviendelangle merged commit 1165f87 into mui:next Sep 23, 2022
@flaviendelangle flaviendelangle deleted the v6.0.0-alpha-0 branch September 23, 2022 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release We are shipping :D
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants