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

[17.0][MIG] web_dialog_size: Migration to 17.0 #2769

Closed

Conversation

jethavadakshal
Copy link

  • Migrated web dialog size module.
  • Removed LegacyComponent as it's deprecated on version 17 and used component instead.
  • Created json controller to get the value of web_dialog_size.default_maximize from system parameters.
  • Removed additional argument from dialog patch.

@pedrobaeza pedrobaeza changed the title 17.0 mig web dialog size [17.0][MIG] web_dialog_size: Migration to 17.0 Mar 14, 2024
@pedrobaeza
Copy link
Member

/ocabot migration web_dialog_size

Please take car about the PR title for being proper, or it won't appear on searches over the module name.

@OCA-git-bot OCA-git-bot added this to the 17.0 milestone Mar 14, 2024
@OCA-git-bot OCA-git-bot mentioned this pull request Mar 14, 2024
26 tasks
@jethavadakshal
Copy link
Author

Hello @pedrobaeza ,
Thanks for your quick response on this PR.
can you please guide me as to why the last 2 checks failed?
Also is there anything missing in this module or is it not properly migrated?

@pedrobaeza
Copy link
Member

Don't worry about the codecov thing. It's just a quality measure about the test coverage. It's not mandatory to comply with it.

Now the PR should be reviewed by people, as stated in https://github.com/OCA/maintainer-tools/blob/master/CONTRIBUTING.md

@jethavadakshal
Copy link
Author

Thanks, @pedrobaeza
The document you provided is really helpful.
From now onward I will take care of the things which are written in documentation while contributing to OCA.

Copy link
Contributor

@CarlosRoca13 CarlosRoca13 left a comment

Choose a reason for hiding this comment

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

Hi @jethavadakshal!

First of all thanks for the work, but it's important to follow the steps defined on https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-17.0 because you have to preserve the commit history.

Please, force push the changes by following this steps 😄

@jethavadakshal
Copy link
Author

Hello @CarlosRoca13,
i have checked the doc shared by you.
and I tried my best to perform the steps.
can you please check and if it's incorrect please guide me on this.
Thanks

@CarlosRoca13
Copy link
Contributor

The steps are:

$ git clone https://github.com/OCA/$repo -b 17.0
$ cd $repo
$ git checkout -b 17.0-mig-$module origin/17.0
$ git format-patch --keep-subject --stdout origin/17.0..origin/16.0 -- $module | git am -3 --keep
$ pre-commit run -a  # to run pre-commit linters and formatters (please ignore pylint errors at this stage)
$ git add -A
$ git commit -m "[IMP] $module: pre-commit auto fixes"  --no-verify  # it is important to do all the formatting in one commit the first time

Then apply your changes and finally commit your changes to force push to this branch. 😄 The importance of doing this is to ensure that the work done by other contributors is not forgotten.

@CarlosRoca13
Copy link
Contributor

And please avoid multiples commits for the migration process to avoid noice, your commits on the migration should be 2:

  • [IMP] $module: pre-commit auto fixes
  • [MIG] $module: Migration to 17.0

@pedrobaeza
Copy link
Member

Superseded by #2919

@pedrobaeza pedrobaeza closed this Aug 16, 2024
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.

5 participants