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

Add forward compatibility for changing HTTPServerRequest from class to struct #2534

Merged
merged 1 commit into from
Feb 20, 2024

Conversation

s-ludwig
Copy link
Member

@s-ludwig s-ludwig commented Feb 9, 2021

No description provided.

@PetarKirov
Copy link
Contributor

@s-ludwig it seems to me that having eventcore, vibe-core and vibe-http as separate repos causes a lot extra work... what do you think about moving them back to the mono repo in the future?

@s-ludwig
Copy link
Member Author

The reasoning for their separation still stands - the core repositories are more or less stable currently and it would be a huge annoyance to bump their major version because of a breaking change in vibe:web or vibe:mongodb. Having them separate is a huge relief w.r.t. advancing their individual APIs.

Friction like the one here is also annoying, but really not that much compared to the alternative.

@s-ludwig s-ludwig force-pushed the vibe-http branch 6 times, most recently from 7a14e86 to cf645e9 Compare February 11, 2021 12:35
@PetarKirov
Copy link
Contributor

Having them separate is a huge relief w.r.t. advancing their individual APIs.

I don't follow. What prevents doing what you're already doing if you have the packages inside the same repo vs in multiple repos? My idea is not to release all the packages in lock step (like we do with vibe:*, but just to host the development of all of them in the same repo. They should still continue to have different (even unrelated release cycles). For example, vibe:http would stay exactly where it is in this repo and then a new folder would be added, e.g. ng/ or whatever that will contain vibe-http, vibe-core, etc., each following its own release model.

@s-ludwig
Copy link
Member Author

The registry doesn't support multiple independent packages inside the same repository, and can't without considerable effort, so that won't AFAICS.

@PetarKirov
Copy link
Contributor

PetarKirov commented Feb 11, 2021

Oh, I wasn't aware of this limitation... it sounds like a deal breaker for monorepo-style development :/ Oh well.
(Basically all of our major projects are structured this way at work using yarn workspaces.)

P.S. I will open an issue for discussion on the dub repo, to not pollute the discussion of this PR further.

@s-ludwig
Copy link
Member Author

It would indeed be nice to support that, but the question is how - by extending the version tag mechanism (e.g. "vibe-d-v1.0.0" and "vibe-http-v1.0.0" instead of just "v1.0.0"), or by storing the version inside dub.sdl and going through the complete git history to determine the corresponding commits? But yeah, let's move that to a dedicated ticket.

@s-ludwig s-ludwig force-pushed the vibe-http branch 4 times, most recently from 32fd996 to 1c93047 Compare February 15, 2024 07:48
@s-ludwig s-ludwig force-pushed the vibe-http branch 2 times, most recently from 2907164 to 8eebfce Compare February 17, 2024 19:42
@s-ludwig s-ludwig changed the title Add optional vibe-http integration Add forward compatibility for changing HTTPServerRequest from class to struct Feb 17, 2024
@s-ludwig
Copy link
Member Author

After the big restructuring, where vibe-http for now is simply the previous state of vibe-d:http master, the only relevant changes left in this branch are a few changes where null is instead written as HTTPServerRequest.init. The rest of the development can then continue in vibe-http.

Makes the code forward-compatible with a switch from class to struct for HTTPServerRequest.
@l-kramer l-kramer merged commit ea777dd into master Feb 20, 2024
58 checks passed
@l-kramer l-kramer deleted the vibe-http branch February 20, 2024 10:39
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