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

[RFC][Urgent] Handling .bolt.yml to bolt.yml migration in 3.2 — Fixing #5894 #5909

Closed
GwendolenLynch opened this issue Oct 16, 2016 · 16 comments
Labels
blocking release RFC Request For Comments
Milestone

Comments

@GwendolenLynch
Copy link
Contributor

GwendolenLynch commented Oct 16, 2016

Ping @bobdenotter @CarsonF @PhillippOhlandt @rossriley @SahAssar et al.

Realistically, this needs to be addressed today as we're plan to tag RC tomorrow and is a simple coding problem to fix, we just need to agree on the approach.

Problem

bolt/composer-install and many/most/all existing 3.x installs have the hidden file. The changes in core are currently half-baked, and non-archive installs will end up with both a hidden an non-hidden file … the hidden file has, and should have, usage priority, and therefore create yet one more thing that will needlessly trip up a significant percentage of our implementer base.

In the case of the Composer target, it should stay hidden, as IMHO this is the correct approach. Making the non-hidden file option available is part of our ongoing effort to bridge the gap between novice and experienced implementers.

We're about to open up a world of embarrassing pain for all of our implementers, in what we believe is going to be the best 3.x minor release branch yet. Let's not fall on our faces here 😉

Proposal

Modify the ScriptHandler post-create-project-cmd callback as follows:

  • Add a question that has the default as the novice friendly option:

"Would you like the load configuration file (bolt.yml) to be hidden? (no): "

  • If the non-hidden option is selected, remove the .bolt.yml file

The problem I foresee with Step 2 is that, you can re-run composer create-project in an existing project root and that will skip straight to executing the post-create-project-cmd callback … and it is easy to miss the questions, as wall-o-text, and nuke your hidden file,

I really think we've completely taken the wrong approach to this problem so far, so can we all have a think about:

a) What problem are we actually trying to solve
b) The negative impacts that we've already created by doing a half arsed job of approaching wrt to this a)
c) How we can achieve a unified agreement on the end goal to providing a solution to a)
d) What is the solution we're going to effect 😆

… aaaaand … Go! 🚦


Linking #5894 and bolt/composer-install#16

@GwendolenLynch GwendolenLynch added the RFC Request For Comments label Oct 16, 2016
@GwendolenLynch GwendolenLynch added this to the Bolt 3.2 - Feature release milestone Oct 16, 2016
@bobdenotter
Copy link
Member

How 'bout we do a check for existing files:

  • If neither .bolt.yml nor bolt.yml exist, create bolt.yml.
  • Else if .bolt.yml exists, rename to bolt.yml.
  • Otherwise leave bolt.yml alone.

@GwendolenLynch
Copy link
Contributor Author

GwendolenLynch commented Oct 16, 2016

If neither .bolt.yml nor bolt.yml exist, create bolt.yml.

Yeah, but this is part of the problem I am highlighting. In a fresh composer create-project the hidden .bolt.yml will always exist, and should (IMHO). Hence why there is no agreement on bolt/composer-install#16 yet.

@GwendolenLynch
Copy link
Contributor Author

Else if .bolt.yml exists, rename to bolt.yml

No, that is meeting the requirements of only 1 group.

@bobdenotter
Copy link
Member

In a fresh composer create-project the hidden .bolt.yml will always exist, and should (IMHO).

Wasn't the idea that that one would change to bolt.yml as well? Not saying it should or shouldn't, just thinking out loud.

@GwendolenLynch
Copy link
Contributor Author

Wasn't the idea that that one would change to bolt.yml as well?

Well, that's a point that I don't think we collectively reached agreement on.

In one corner are the people who want that, and think it is the correct approach, in other are the people who use FTP GUIs to upload files and get burnt by the lack of transparency in the process … and end up with a broken install.

The other problem is that the approach currently is a big BC break, and I haven't been able to think of a good middle ground (but we really should not have done this either way, but let's get this over the line regardless).

To expand on

Else if .bolt.yml exists, rename to bolt.yml.

Thinking out loud myself … I am concerned that this starts cross over (IMO … and I've regularly been known to be very wrong) to dictating an established approach, in a minor release, to one target group … the group that is heavily packed with grumpy OCD 🐨 types 😆

@bobdenotter
Copy link
Member

to dictating an established approach, in a minor release

Another random suggestion: Maybe not fix it, but deprecate the use of .bolt.yml to be removed in 4.0? In the mean time, we'll make do with extra ugly checks.

@rossriley
Copy link
Contributor

If we support checking for both, then there's no risk of a BC break is there, as in if you have an existing project with .bolt.yml and you upgrade, nothing will change and nothing will break.

Then in 4.0 we have .bolt.yml as a breaking change and just remove the check.

So personally I'd just skip the middle step of bob's first reply, don't do a rename.

@GwendolenLynch
Copy link
Contributor Author

If we support checking for both, then there's no risk of a BC break is there

The BC break is that we end up with two files, and one has to be read first.

It is not unheard of to re-trigger the callback, and after testing it this morning, it is pretty easy to get conflicting files and it not be obvious when looking via ls or via a file manager.

Given that 3.2.0 will be the 20th stable release of the 3.x series, we've got a lot locked in already.

This is that the base file should be in the composer-install repo, hidden or not, scripts can, and do fail, and sometimes that is Composer's fault, sometimes Bolt's. Having a fallback that reflects the default install is very Bolt 😄

Then in 4.0 we have .bolt.yml as a breaking change and just remove the check.

I personally don't want to see that … but 4.0 should be flexible enough that I can just run a fork to meet my personal needs … but having to do that kinda makes me 😿

😆

@rossriley
Copy link
Contributor

How would we get two files? if they have a .bolt.yml then nothing else happens, if they don't have either we create bolt.yml for them, if they happened to create two files manually somehow or merged installs or something, then we can assume bolt.yml is the correct file to read, since the existence of that means they've run an install post 3.2 and that then becomes higher priority.

In terms of what we do for 4.0 that can be a separate discussion, I just assumed it would be simpler to just have one way to do it rather than supporting both.

@bobdenotter
Copy link
Member

Don't want to get sidetracked here, but:

I personally don't want to see that … but 4.0 should be flexible enough that I can just run a fork to meet my personal needs …

Having to run off a fork should never be the preferred workflow, regardless of how "easy" it would be to do so. :-)

@GwendolenLynch
Copy link
Contributor Author

The first thing create-project does is to clone bolt/composer-install and as mentioned that has a .bolt.yml file to ensure there is always a fallback.

This is why I call the current 3.2 approach half-arsed … it only does half the job, and is really easy to fix that part 😄

Where the problem comes is that we're in stable/minor cycle with 19 stable releases, and it is completely reasonable for someone to expect to continue to use the hidden version.

The balance to strike is what to do, esp. when scripts are re-triggered … I actually was testing something out and raised this RFC when I was able to screw up a local install 😆 … 🐨ed myself all by myself.

In terms of what we do for 4.0 that can be a separate discussion, I just assumed it would be simpler to just have one way to do it rather than supporting both.

Totally on board with that 👍

@bobdenotter
Copy link
Member

Should we close this one, because of the merged 'patch', or should we leave this open as an [RFC] to tackle it better for 4.0?

@GwendolenLynch
Copy link
Contributor Author

4.0 is still a bit away … and those advocating a better way to handle it are not likely to forget in the mean time, so I vote for …

@GwendolenLynch
Copy link
Contributor Author

Just adding a note regards this … from the 14th of June 2012 … We've always had hidden files, that to the target group that this problem was highlighting, is potentially critical failure inducing 😉

@bobdenotter
Copy link
Member

And .htaccess has been a point of failure to novices for over a decade now. If it were up to me we'd get rid of that too. ;-)

@GwendolenLynch
Copy link
Contributor Author

Yes, but if you're going to need to fix one, the we need to fix the other first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocking release RFC Request For Comments
Projects
None yet
Development

No branches or pull requests

3 participants