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.

Sign up for GitHub

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

Jump to bottom

#1216 #1955 #1956 Overloaded AddOcelot method to support merging of configuration files to memory #1227

Merged
merged 19 commits into from
Mar 15, 2024
Merged

#1216 #1955 #1956 Overloaded AddOcelot method to support merging of configuration files to memory #1227

merged 19 commits into from
Mar 15, 2024

Conversation

ebjornset
Copy link
Contributor

@ebjornset ebjornset commented May 10, 2020 •
edited by raman-m
Loading

@ebjornset
Copy link
Contributor Author

ebjornset commented Jun 11, 2020 •
edited
Loading

Hi. I just realized that I've not linked this PR correctly to issue #1216, but it seems that I don't have permissions to add the issue link?

stefancruz added a commit to stefancruz/Ocelot that referenced this pull request Dec 22, 2022
@stefancruz
Merge branch 'feat/1216-Merge-ocelot-json-to-memory'
12e73f3 
Conflicts:
	docs/features/configuration.rst
	src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs
	test/Ocelot.AcceptanceTests/Steps.cs
	test/Ocelot.UnitTests/DependencyInjection/ConfigurationBuilderExtensionsTests.cs

	Cherry picked from ThreeMammals#1227
@raman-m raman-m changed the title Added new ConfigurationBuilderExtensions.AddOcelot overload to support merging of configuration files to memory #1216 AddOcelot overload to support merging of configuration files to memory Jul 19, 2023
@raman-m raman-m changed the base branch from master to develop July 19, 2023 19:28
@raman-m raman-m added the conflicts Feature branch has merge conflicts label Jul 19, 2023
@raman-m
Copy link
Member

raman-m commented Jul 19, 2023

@ebjornset
Hi Eirik!
Thanks for the great PR!

Unfortunately It is hard to resolve merge conflicts using GitHub UI.
Could you resolve merge conflicts please?
You need to merge develop branch to feature branch with rebasing. See below! 👇

And I don't see develop branch in your fork!
Could you Sync fork please? So, new develop branch will occur with all top commits!

Let me know upgrading results please!

@raman-m raman-m force-pushed the feat/1216-Merge-ocelot-json-to-memory branch from 8c4e963 to 694fd9e Compare August 1, 2023 11:41
@raman-m raman-m added bug Identified as a potential bug feature A new feature needs feedback Issue is waiting on feedback before acceptance and removed conflicts Feature branch has merge conflicts labels Aug 1, 2023
@raman-m
Copy link
Member

raman-m commented Aug 1, 2023 •
edited
Loading

Eirik,
I thought you need some help in merge conflicts resolving.
There are no conflicts now.
The feature branch has been rebased onto ThreeMammals:develop.

And we can start code review...

Also, your forked repo is too old. There is no develop branch!

image

Could you add me as collaborator to your forked repo please? I will create develop branch and make it default.

@raman-m
Copy link
Member

raman-m commented Aug 1, 2023 •
edited
Loading

@pablo0219
Could you review the code please?
Your opinion is prioritized due to past discussions.

@raman-m raman-m mentioned this pull request Aug 1, 2023
Closed
raman-m
raman-m requested changes Aug 3, 2023
View reviewed changes
Copy link
Member

@raman-m raman-m left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@ebjornset
I've pointed my attention to these new extensions because they are highly reused by developers. These extension-methods are critical in Ocelot core.
And, it makes sense to develop them a bit more.

Could you fix the issues please? 👇

docs/features/configuration.rst Outdated Show resolved Hide resolved
docs/features/configuration.rst Outdated Show resolved Hide resolved
docs/features/configuration.rst Outdated Show resolved Hide resolved
src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs Outdated Show resolved Hide resolved
src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs Outdated Show resolved Hide resolved
src/Ocelot/DependencyInjection/MergeOcelotJson.cs Show resolved Hide resolved
test/Ocelot.AcceptanceTests/ConfigurationMergeTests.cs Outdated Show resolved Hide resolved
test/Ocelot.AcceptanceTests/ConfigurationMergeTests.cs Outdated Show resolved Hide resolved
test/Ocelot.AcceptanceTests/Steps.cs Outdated Show resolved Hide resolved
test/Ocelot.UnitTests/DependencyInjection/ConfigurationBuilderExtensionsTests.cs Outdated Show resolved Hide resolved
@raman-m raman-m removed the needs feedback Issue is waiting on feedback before acceptance label Mar 8, 2024
@raman-m raman-m changed the title #1216 AddOcelot overload to support merging of configuration files to memory #1216 #1955 #1956 AddOcelot overloaded method to support merging of configuration files to memory Mar 8, 2024
@raman-m raman-m linked an issue Mar 8, 2024 that may be closed by this pull request
Access to the path '/app/ocelot.json' is denied #1955
Closed
@raman-m raman-m changed the title #1216 #1955 #1956 AddOcelot overloaded method to support merging of configuration files to memory #1216 #1955 #1956 Overloaded AddOcelot method to support merging of configuration files to memory Mar 8, 2024
raman-m
raman-m approved these changes Mar 11, 2024
View reviewed changes
Copy link
Member

@raman-m raman-m left a comment •
edited
Loading

Choose a reason for hiding this comment

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

TODO Docs

ggnaegi
ggnaegi approved these changes Mar 12, 2024
View reviewed changes
Copy link
Member

@ggnaegi ggnaegi left a comment

Choose a reason for hiding this comment

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

Ok, thanks.

@raman-m raman-m removed the bug Identified as a potential bug label Mar 12, 2024
@raman-m
Copy link
Member

raman-m commented Mar 14, 2024 •
edited
Loading

Ready for delivery! ✅

  • Code review ✔️✔️
  • Team approvals ✔️
  • Unit testing ✔️
  • Acceptance testing ✔️
  • Reviewed and updated docs ✔️✔️

@raman-m
Update configuration.rst
b60cde7 
Replace "kind" with "type"
@raman-m raman-m merged commit 84bd6e4 into ThreeMammals:develop Mar 15, 2024
2 checks passed
@raman-m raman-m mentioned this pull request Mar 16, 2024
Closed
@raman-m raman-m mentioned this pull request Mar 29, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wast wast wast approved these changes

@raman-m raman-m raman-m approved these changes

@ggnaegi ggnaegi ggnaegi approved these changes

@sharpn sharpn Awaiting requested review from sharpn

@RaynaldM RaynaldM Awaiting requested review from RaynaldM

Assignees

@ebjornset ebjornset

Labels
Configuration Ocelot feature: Configuration feature A new feature Feb'24 February 2024 release
Projects
None yet
Milestone
February'24
5 participants
@ebjornset @raman-m @RaynaldM @wast @ggnaegi