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

Compatibility with Nette 3.0 #7

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

PavelJurasek
Copy link
Contributor

in case you were interested

@PavelJurasek PavelJurasek force-pushed the v2.0 branch 2 times, most recently from aa55c8b to 9b4c919 Compare April 7, 2019 10:23
@mestrode
Copy link

mestrode commented Feb 9, 2022

What do we need to proceed here?

@PavelJurasek
Copy link
Contributor Author

rebased onto master, not much more I can do about it

@MartinMystikJonas
Copy link
Contributor

I will check it tomorrow

@MartinMystikJonas
Copy link
Contributor

MartinMystikJonas commented Feb 9, 2022

Could you just revert formatting and codding standard changes back to original so I can check what exactly changed? Right now diff shows that entire files changed because there are changes in indent, brackets, etc.

MetisFW uses own coding standard so updating this library to another (Nette?) alongside other changes is not desired.

@MartinMystikJonas
Copy link
Contributor

@PavelJurasek Thanks your your PR. I should update it long ago myself. tried to do review your PR but it is really hard when significant changes are mixed with irrelevant formating changes in single commit. Unfortunatelly I am under hight time pressure at the moment to untangle it.

Could you either create new PR with only significant changes (without any changes of formating and coding standard) or write me a summary of required changes for Nette 3.0 compatibility so I can prepare new release myself?

Thanks.

@MartinMystikJonas
Copy link
Contributor

@mestrode I will try to make new release soon. Unfortunatelly work and personal time requirements does not leave me with much time to maintain all thing I should. Sorry about that.

@mestrode
Copy link

I'd like to make things easy, open an second pull request (since I don't know how to this within this pull request)

  1. I'll use prettier; This should not change nothing functional
  2. I'll try to cut the changes into smaller changes. Do I have to do this within an second pull request?

please help me out, when I got sucked...

@MartinMystikJonas
Copy link
Contributor

@mestrode Well point is that MetisFW and all our projects uses it's own coding standard and I would like to keep it that way instead of switching to Nette/Prettier style if possible.

So ideal solution would be to revert fromatting changes done in this PR and keep only functional changes. If you would like to help then I suggest forking from @PavelJurasek last commit, reverting formatting changes and creating new PR which would be easier to review and would not change formatting.

I can share you our PhpCS config used for internal projects if you would like to use it to revert changes. Unfortunatelly I never had time to polish and publish it as part of public MetisFW yet. But I can send you "work in progress" version extracted from our build tools.

@MartinMystikJonas
Copy link
Contributor

@mestrode
Copy link

hmm. Honestly I'm a Nette novice and do barely understand all the details of all the code and also tools.
Thank you for your coding standard. But actually I don't know how to apply this.

The code from PavelJurasek was more or less easy to divide in separate changes. You can take look into the second commit, I've prepared. If you like to have things different, please let me know.

@MartinMystikJonas
Copy link
Contributor

@mestrode Ok never mind. Your following commits looks much easier to review. I will go throught it and prepare new release myself based on thes changesets. I hope it would be some time next week but I cannot promise that.

@MartinMystikJonas
Copy link
Contributor

@PavelJurasek @mestrode Guys could you check branch v2.x (dev-v2.x) in your projects? I created new branch based on changes by @PavelJurasek but I had to re-apply them manually as part of review process so it is not exactly 1:1. It if would be ok I will release version 2.0

@mestrode
Copy link

mestrode commented Mar 1, 2022

@MartinMystikJonas Thank you. I have already started a new fork (and have understood how this could be done more easy, next time).

I will definitely check your new version, since I need a payment solution within my project. But at the moment there is one other tasks I have to complete first. So I'll come back to you soon.

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