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

Fixed 'duplicate extension' issue #1191

Merged
merged 4 commits into from Feb 3, 2023

Conversation

priytosh-tripathi
Copy link
Contributor

Commented out 'RotateText' extension to fix 'Duplicate extension' issue. The former extension is commented out to keep it as an example of illustrating the 'Minimum code required to create an extension with static values.'

@hvbtup
Copy link
Contributor

hvbtup commented Jan 26, 2023

@priytosh-tripathi Please sign the ECA (Eclipse Contributor Agreement)

@priytosh-tripathi
Copy link
Contributor Author

@priytosh-tripathi Please sign the ECA (Eclipse Contributor Agreement)

@hvbtup I have signed the ECA now.

@wimjongman wimjongman changed the title Fixed 'duplicate extension' isuue Fixed 'duplicate extension' issue Jan 26, 2023
@hvbtup
Copy link
Contributor

hvbtup commented Jan 26, 2023

Did you test this? To me it seems like you just crippled both versions of the extension.
I think resolving this issue requires at least removing the lines referencing ""org.eclipse.birt.report.extension.tutorial_1" from the *.launch files (aka run configurations). This is to avoid the issue when we call the BIRT IDE directly from the Build IDE .
But furthermore it is also necessary to exclude them from the Maven build somehow.

@priytosh-tripathi
Copy link
Contributor Author

Did you test this? To me it seems like you just crippled both versions of the extension. I think resolving this issue requires at least removing the lines referencing ""org.eclipse.birt.report.extension.tutorial_1" from the *.launch files (aka run configurations). This is to avoid the issue when we call the BIRT IDE directly from the Build IDE . But furthermore it is also necessary to exclude them from the Maven build somehow.

I need a little clarity over the proposed solution, I can see three of the possible solution I deduced,

  1. We need to remove one of the extensions (static one) from both extension-tutorial-1 and extension-tutorial-2 ?
  2. We need to remove one of the tutorial (probably tutorial_1 from the build) and the left over tutorial (tutorial_2 based on this statement) will have only one of the extension.
  3. We need to remove only one of the tutorial (tutorial_1) and let the other tutorial (tutorial_2) to stay in the build and other references?

Also, is there some Slack channel / Teams Channel by which we can connect better ?

@hvbtup
Copy link
Contributor

hvbtup commented Jan 27, 2023

extension-tutorial-1 is only useful as a minimal example, but not suitable for real-world use.

extension-tutorial-2 is more complicated (even more than necessary, because the implementation contains two alternative ways), but it is good enough to be used in production.

What I mean is: Both versions should be kept in the sources, but only extension-tutorial-2 should make it into the generated artifacts and into the .launch files.

Copy link
Contributor Author

@priytosh-tripathi priytosh-tripathi left a comment

Choose a reason for hiding this comment

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

hi @hvbtup ,

Please suggest if these are the required changes? I am also facing a little issue due to multiple email-id, ao I might have to open a new PR for the changes to merge once done.
Please review this and suggest changes.

@priytosh-tripathi
Copy link
Contributor Author

Hi @hvbtup

I could not find any more references for the files to be removed. Please suggest if I am missing out on any of the step.

Thanks,

@priytosh-tripathi
Copy link
Contributor Author

Hi @hvbtup ,

I have successfully resolved the multiple account ETA conflict issue, we can work on the issue seamlessly now!

@hvbtup
Copy link
Contributor

hvbtup commented Jan 30, 2023

Thx, I'll try to look at it this week

@hvbtup hvbtup merged commit 24af10e into eclipse-birt:master Feb 3, 2023
@hvbtup
Copy link
Contributor

hvbtup commented Feb 3, 2023

Thanks, Priytosh!

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.

None yet

3 participants