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

Bring DataStorm into Ice 3.8 #2902

Merged
merged 13 commits into from
Oct 17, 2024
Merged

Bring DataStorm into Ice 3.8 #2902

merged 13 commits into from
Oct 17, 2024

Conversation

pepone
Copy link
Member

@pepone pepone commented Oct 16, 2024

The code comes straight from datastorm/3.8 branch with a few minor updates:

  • Updated the build to use project rules from Ice repository.
  • Updated the tests scripts to use scripts/ from Ice repository.
  • Fixed new build warnings that show up as a result of the updated build system, nothing major.
  • Added handling of AdapterDestroyedExeption, which was not done in datastorm/3.8

#ifndef DATASTORM_CONFIG_H
#define DATASTORM_CONFIG_H

#include "Ice/Config.h"
Copy link
Member Author

Choose a reason for hiding this comment

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

Simplified this file to align with other Xxx/Config.h in this repository.

template<typename Key, typename Value, typename UpdateTag>
void Reader<Key, Value, UpdateTag>::waitForWriters(unsigned int count) const
{
_impl->waitForWriters(static_cast<int>(count));
Copy link
Member Author

Choose a reason for hiding this comment

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

This static_cast is new, I have to add it because the signed/unsigned mismatch. There are a few others for other waitForXxx methods

props = Client.getProps(self, current)

# Default properties
props.update(
Copy link
Member Author

Choose a reason for hiding this comment

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

I added this default properties here, in the datastorm repository they were part of the DataStorm component class.

https://github.com/zeroc-ice/datastorm/blob/fba29e653d53d3493c423ad1f853f4bee15952cd/scripts/Component.py#L96

The component also provide a --multicast option that I haven't yet add here.

I would deal with this in a follow up PR.

#ifndef DATASTORM_DATASTORM_H
#define DATASTORM_DATASTORM_H

#include "DataStorm/Config.h"
Copy link
Member

Choose a reason for hiding this comment

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

Should be "Config.h", "Node.h" etc.

module DataStorm
{

/**
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice to indent this file correctly.


namespace DataStorm
{

Copy link
Member

Choose a reason for hiding this comment

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

We should clean-up all these extra empty lines in a follow-up PR.

@pepone pepone merged commit 8e3d0b0 into zeroc-ice:main Oct 17, 2024
14 of 17 checks passed
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