Skip to content
This repository has been archived by the owner on Oct 30, 2019. It is now read-only.

Fix issues with namespace clashing in our interal deployer #344

Merged
merged 4 commits into from
Mar 21, 2017

Conversation

Jw0x47
Copy link
Contributor

@Jw0x47 Jw0x47 commented Mar 20, 2017

@gchomatas

Our internal fabric deploy was preventing us from deploying this because there was a config yaml namespace conflict on the key github.

Rather than deal with more config clashes in the future I opted to re-name all the blazar specific options to blazar#### so that when reading the config it is clear what properties are for the deployer and which are consumed by Blazar.

@gchomatas
Copy link
Contributor

@jonathanwgoodwin why not just have all blazar settings under a blazar: property?
i.e. the top level config has only a blazar property which in turn includes all other settings? This seems to be a cleaner and more elegant solution for us and for those that don't mix the dw config with their deploy config.

@Jw0x47
Copy link
Contributor Author

Jw0x47 commented Mar 21, 2017

@gchomatas I've updated the PR to nest the blazar configuration inside a single property blazarConfiguration.

@gchomatas
Copy link
Contributor

🚢

@hs-jenkins-bot
Copy link
Contributor

gchomatas's ship has signed off 4 commits by jgoodwin.
This PR is now reviewed. :shipit:

@Jw0x47 Jw0x47 merged commit a810f99 into hs-master Mar 21, 2017
public class BlazarConfiguration extends Configuration {
/**
* The configuration for Blazar.
* All options that control how the BlazarService behaves are configured here. These are wrapped by {@Link BlazarWrapperConfiguration}

Choose a reason for hiding this comment

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

s/BlazarWrapperConfiguration/BlazarConfigurationWrapper ?

@Jw0x47 Jw0x47 deleted the namespace-config branch March 21, 2017 19:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants