Skip to content
This repository was archived by the owner on Nov 10, 2025. It is now read-only.

Fix application logs after adding EventStream usage and OnPluginsLoaded.#55

Merged
jsolman merged 11 commits intoplugins-2.10.1from
FixSubscribeAppLogsOrder
Mar 14, 2019
Merged

Fix application logs after adding EventStream usage and OnPluginsLoaded.#55
jsolman merged 11 commits intoplugins-2.10.1from
FixSubscribeAppLogsOrder

Conversation

@jsolman
Copy link
Copy Markdown
Contributor

@jsolman jsolman commented Mar 12, 2019

Closes #50

@vncoelho
Copy link
Copy Markdown
Member

Sounds good, Jeff, I will soon take a look.

@jsolman jsolman requested a review from erikzhang March 12, 2019 02:05

<ItemGroup>
<PackageReference Include="Neo" Version="2.9.4" />
<PackageReference Include="Neo" Version="2.10.1-CI00001" />
Copy link
Copy Markdown
Member

@erikzhang erikzhang Mar 12, 2019

Choose a reason for hiding this comment

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

Should wait for the release of neo core.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That is fine, this will just stay here for reference for people building master that need this plugin to work, right?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi @erikzhang,
Do you think that we should create another branch, for example, "plugins dev" and merge this and other upcoming changes there?
In this sense we could aggregate everything that is needed for the next release.

Copy link
Copy Markdown
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

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

It looks good, let's wait for the next release.

vncoelho
vncoelho previously approved these changes Mar 13, 2019
Copy link
Copy Markdown
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

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

Tested locally and plugin was working as expected with the last up to date neo master.

@jsolman jsolman dismissed erikzhang’s stale review March 13, 2019 20:51

Old review comments were addressed.

@jsolman
Copy link
Copy Markdown
Contributor Author

jsolman commented Mar 13, 2019

This also still references NuGet CI build, thus waiting to merge.

@erikzhang erikzhang dismissed vncoelho’s stale review March 14, 2019 04:03

CI NuGet reference

@vncoelho
Copy link
Copy Markdown
Member

closes #50

@jsolman jsolman changed the base branch from master to plugins-2.10.1 March 14, 2019 18:21
@jsolman jsolman merged commit ae84ce4 into plugins-2.10.1 Mar 14, 2019
@vncoelho vncoelho deleted the FixSubscribeAppLogsOrder branch March 14, 2019 18:25
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.

3 participants