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

Alpha release? #181

Closed
mgrip opened this issue Mar 8, 2018 · 34 comments
Closed

Alpha release? #181

mgrip opened this issue Mar 8, 2018 · 34 comments
Labels

Comments

@mgrip
Copy link
Contributor

mgrip commented Mar 8, 2018

#174 (comment)

Wanted to start a discussion on where people think the quality of this plugin is currently, and plans for rolling out more widely.

For myself I've been running --debug-check on a codebase of ~1000 php files, and as of this morning am not getting any breaking errors 🎉

Do people think its time to start publishing this as an alpha/beta release to encourage more people to start using and finding edge cases? I'm guessing that would mostly just entail bumping our package version to 0.1.0 and updating our README.md + documentation

@mgrip mgrip added the question label Mar 8, 2018
@czosel
Copy link
Collaborator

czosel commented Mar 8, 2018

I just tried the same with my codebase from work, and for me #112 seems to be an issue at several places (can't quite tell yet if that is the only problem). I'd suggest not doing a formal release before we've verified that we don't have breaking AST changes for a couple of larger codebases (might be interesting to try some larger open source projects).

Of course I'm always in favor of encouraging people to try prettier for php on their projects and report back issues 😄 We just have to careful not to give the impression that the plugin is ready to be used, because then people might loose trust when we break their code (prettier/prettier#3521 (comment)).

@alexander-akait
Copy link
Member

In end of month i think we can release alpha 👍

@alexander-akait
Copy link
Member

I think we will be the first of the official plugins 🥇

@mgrip
Copy link
Contributor Author

mgrip commented Mar 19, 2018

I think several of the largest remaining issues are solved with #217 (#190, #191, #173). Seems to me like getting that PR over the line will be the majority of what we need to get this to v0.1.0

@aboyton
Copy link

aboyton commented Mar 25, 2018

I'm guessing that if I run --debug-check and there's a bug with the AST generation itself then this won't detect it? I'm guessing this isn't particularly likely but could happen.

I'm very happy to help with testing on the reasonably large PHP codebase that I have at work. Sadly I got stuck on my first file (see #29, #252, #253, #254) but I'm really exited by this.

@mgrip
Copy link
Contributor Author

mgrip commented Mar 30, 2018

I believe all of the high priority issues mentioned above have now been fixed

@alexander-akait
Copy link
Member

We have big problem with mixed contents, we should fix it and we can alpha release. Try to run plugin on any WordPress theme repo

@mgrip
Copy link
Contributor Author

mgrip commented Mar 30, 2018

@evilebottnawi do we already have an open issue for that? Currently the only open issue we have labeled as high priority is #267

@czosel
Copy link
Collaborator

czosel commented Mar 30, 2018

@evilebottnawi You mean PHP embedded in HTML? To me, something that only works on .php files without mixed content would also be a very valuable starting point. I haven't looked at the HTML printer yet, but I could imagine that proper support for PHP in HTML would require a pretty solid foundation on the HTML side.

@aboyton
Copy link

aboyton commented Mar 31, 2018

As I mentioned before, I’m happy to give this a go on more code before an alpha if you’d like over the next 3 days (completely up to you obviously, I don’t want to delay you unnecessarily). So far I’ve only tested Prettier on a single PHP file and all the issues I’ve reported were from that (which I’m unsure what that says about my PHP code).

@aboyton
Copy link

aboyton commented Mar 31, 2018

The other thing you could consider if finding a couple of big open source PHP code bases and running Prettier over them. That worked well for the JavaScript version IIRC.

@czosel
Copy link
Collaborator

czosel commented Mar 31, 2018

@aboyton Testing and reporting issues is always appreciated and will speed us up instead of slowing us down! 😉
I think it's really important that we're confident about not breaking any code before we do our first release.

@mgrip
Copy link
Contributor Author

mgrip commented Apr 2, 2018

@vjeux @azz I think we're getting close to fixing the last of any breaking issues. When we get to the point that we think this is good to move out the development release (hopefully within a week?), what work do you guys think would be involved in releasing this as an alpha/beta? Just bumping the npm version? I think @azz is the owner of the module and might have to do that

@vjeux
Copy link
Contributor

vjeux commented Apr 2, 2018

Let me know what your npm names are and I can add you to @prettier/prettier-php. I'm not really sure why prettier-php has been deprecated (and I don't have access).

I think that publishing a new version is what is needed technically. I would recommend making a blog post / release notes about it so that we can tweet about it from the prettier account to get visibility.

@ichiriac
Copy link

ichiriac commented Apr 3, 2018

Hi guys, congrats for the amazing job done !

As the final 3.0 is not ready yet, I could also make a pre-release of php-parser (3.0.0-pre1) that includes major bug fixes - do you see any blocking issue that I could prioritize ?

@alexander-akait
Copy link
Member

@ichiriac Thanks for work on php parser, only one strange error #313, maybe parser problem

@czosel
Copy link
Collaborator

czosel commented Apr 3, 2018

@vjeux shouldn't it be @prettier/plugin-php for consistency? I'm also not sure about the naming conventions, @azz might now more. Also:

  • my npm handle is czosel
  • I could draft a blog post on the weekend!

@alexander-akait
Copy link
Member

I agree we should be @prettier/plugin-php for official consistently naming

@mgrip
Copy link
Contributor Author

mgrip commented Apr 3, 2018

  • +1 for consistency
  • my npm name is mgrip
  • thanks @czosel! let us know if you want another pair of eyes

@czosel
Copy link
Collaborator

czosel commented Apr 3, 2018

@mgrip I'll post a draft on GitHub as soon as I have something 😄

@vjeux
Copy link
Contributor

vjeux commented Apr 3, 2018

Okay, I have no idea how I can add you in the scoped module :x

Trying to add you from here https://www.npmjs.com/package/@prettier/plugin-php/access gives me this error message:

image

Trying from the command line gives me a similar error:

npm team ls @prettier/plugin-php
npm ERR! code E404
npm ERR! 404 Not Found : -/org/%40prettier%2Fplugin-php/team

@azz do you know what's going on?

@azz
Copy link
Member

azz commented Apr 4, 2018

Looks like the npm backend is having a bad time.

image

image

@azz
Copy link
Member

azz commented Apr 4, 2018

Ok got it sorted:

  • Created a team called php in the prettier org.
  • Added the @prettier/plugin-php package to the php team.
  • Added @czosel and @mgrip as members of the prettier org and the php team.

You both should have publish access for this package now.

@czosel
Copy link
Collaborator

czosel commented Apr 4, 2018

Looks like it's working - thanks @azz!

@czosel
Copy link
Collaborator

czosel commented Apr 6, 2018

For the blog post I'd really like to include some instructions on how to get this to run in an editor. I've looked both at prettier-vscode (prettier/prettier-vscode#395) and at vim prettier (prettier/vim-prettier#119), but both don't seem to work with plugins yet. Has anyone of you had success with an editor already? @mgrip @evilebottnawi

@alexander-akait
Copy link
Member

@czosel i use intellij idea, he is support prettier, also have external tool to support any tools

@alexander-akait
Copy link
Member

@czosel @mgrip before alpha release we should solve #156, feel free to finish this PR and release alpha

@czosel
Copy link
Collaborator

czosel commented Apr 7, 2018

@evilebottnawi does the PHP plugin work in IntelliJ already?

@alexander-akait
Copy link
Member

@czosel yes, just install prettier and plugin-php and all works fine

@tuananh
Copy link

tuananh commented Apr 7, 2018

Thank you for all the hard works <3

@czosel
Copy link
Collaborator

czosel commented Apr 7, 2018

@evilebottnawi Wow, pretty cool!

About the peer dependency: Is it really important that this lands before releasing? I don't really see why.
We should, however, update the install command in the README to just yarn add --dev prettier/plugin-php if we decide to release it without making prettier a peer dependency, right?

@alexander-akait
Copy link
Member

alexander-akait commented Apr 7, 2018

@czosel have two version prettier is very bad, also if you don't use > npm@4 your can got error because peer deps can be on top in node_modules, also it is increase node_modules size, because prettier have pinned deps and our version prettier have other pinned deps.

@czosel
Copy link
Collaborator

czosel commented Apr 7, 2018

@evilebottnawi Alright - please see my comment in #156. Edit: Oops, you did already 😄

@mgrip
Copy link
Contributor Author

mgrip commented Apr 9, 2018

Deployed!

@mgrip mgrip closed this as completed Apr 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants