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

Change Adapter oM and Engine namespaces to Adapters #213

Closed
14 of 23 tasks
alelom opened this issue Mar 10, 2020 · 18 comments
Closed
14 of 23 tasks

Change Adapter oM and Engine namespaces to Adapters #213

alelom opened this issue Mar 10, 2020 · 18 comments
Labels
type:compliance Non-conforming to code guidelines

Comments

@alelom
Copy link
Member

alelom commented Mar 10, 2020

Description

See the general issue text below.

Action required

The repos that need attention are all Toolkits that include an Adapter project.

@alelom alelom added the type:compliance Non-conforming to code guidelines label Mar 10, 2020
@alelom
Copy link
Member Author

alelom commented Mar 10, 2020

Description

We want the menu items for Engine and oM classes defined in Adapter-Based-Toolkits (= only Toolkits that contain an Adapter project) into a new category, Adapters.

This is mainly to tidy up the menu items, to avoid clutter, and leaving the base Engine methods and classes on the top level.

Currently we have this situation:

What we want is:

This means:

  1. Take note of all changes you will be doing at the following points.
  2. Add the Adapters name to the namespace of all your oM and Engine classes.
  3. This last point applies only for some Toolkits such as Lusas and Revit in the first picture above. Check if you have:
    • BH.oM.Adapter.something have to become BH.oM.Adapters.something (note the s);
    • BH.oM.External.something have to become BH.oM.Adapters.something.
    • BH.oM.Adapters.something do not change them.
    • BH.Engine.Adaper.something have to become BH.Engine.Adapters.something (note the s);
    • BH.Engine.External.something have to become BH.oM.Adapters.something.
    • BH.Engine.Adapers.something do not change them.

You must take note of what changes you've applied and list them in your PR.

E.g.

  • All classes that were originally under BH.oM.SAP2000 are now into BH.oM.Adapers.SAP2000
  • All classes that were originally under BH.Engine.SAP2000 are now into BH.Engine.Adapers.SAP2000
  • All classes that were originally under BH.oM.Adapter.SAP2000 are now into BH.oM.Adapers.SAP2000 (note the s)
  • All classes that were originally under BH.oM.External.SAP2000 (note the s) are now in BH.oM.Adapers.SAP2000

this will be used to inform the Versioning_Toolkit and make sure we get retro-compatibility for this change.

IMPORTANT NOTE

Note that the following usings:

using BH.oM.Adapter;
using BH.oM.Adapter.Commands;
using BH.Engine.Adapter;
using BH.Adapter.Modules.Structure

are not to be changed as they refer to the base Adapter namespace, which is not the subject of the current renaming.

@alelom alelom added this to the BHoM 3.2 β MVP milestone Mar 11, 2020
@alelom
Copy link
Member Author

alelom commented Mar 11, 2020

After last chat with @al-fisher

We decided to bump this as a priority in 3.2

Reason is, to get best compromise between the benefit of the renaming and the actual amount of work, we thought it's best to properly change all existing Adapter-based-Toolkits.

I'm including updated instructions above. I've tried to be as clear as possible. Let me know what you think.
@FraserGreenroyd @IsakNaslundBh

@rwemay
Copy link
Member

rwemay commented May 13, 2020

Apart from the vote for BH.oM.Adapters (plural), I'm just going to throw one final curved ball in there - BH.oM.Adaptation.

@IsakNaslundBh
Copy link
Contributor

My vote goes to BH.oM.Adapters

@epignatelli
Copy link
Member

I also updated the wiki for the External class to specify what that is about:
https://github.com/BHoM/documentation/wiki/BHoM_Engine-Classes/

My vote goes to Adaptation.

@alelom
Copy link
Member Author

alelom commented May 13, 2020

Everyone,
thanks for the nice chat!

To recap:

  • We agreed that External as a name is more suited for @epignatelli 's use case. His use case didn't exist when we originally had agreed on that name here. To avoid confusion, we had to reconsider other options.
  • I proposed Toolkit instead, but there was general consensus against this – @al-fisher clearly formulated how this would go against BHoM principles
  • We ended up back on considering Adapter, but again with no consensus, mainly because that creates confusion: having BH.oM.Adapter / BH.Engine.Adapter is confusing for people less familiar with the framework ("are the classes in those namespaces actually contained in the Adapter project or in a oM or Engine project")?
  • After some conversation we went back considering Adapters (with an s) to distinguish namespaces from the actual project. Resulting proposal: BH.oM.Adapters / BH.Engine.Adapters
  • @alelom, @rwemay and @epignatelli still were not very convinced – more distinction from the Adapter project was perceived as needed
  • @rwemay proposed Adaptation as a candidate. Resulting proposal: BH.oM.Adaptation / BH.Engine.Adaptation

We'd like everyone to express their preference between the 2 remaining proposals in this survey:
https://www.surveymonkey.co.uk/r/BHGQYX3

Thanks!

@pawelbaran
Copy link
Member

True democracy!

@rwemay
Copy link
Member

rwemay commented May 13, 2020

:) I bottled it and went with my initial instinct of adapters plural. Looks like adapters wins?! Let’s reserve adaptation for truly chameleon like behaviours!

@alelom
Copy link
Member Author

alelom commented May 14, 2020

image
Official results for the joy of democratation 😄

@alelom alelom changed the title Change Adapter oM and Engine namespaces to External Change Adapter oM and Engine namespaces to Adapters May 14, 2020
@alelom
Copy link
Member Author

alelom commented May 14, 2020

Updated issue title and instructions.

@FraserGreenroyd
Copy link
Contributor

Can I confirm that this is now the absolute latest version that we're going with? Cause XML Toolkit has already changed to External and will now need updating again, and MidasCivil/Lusas have gone to External by @peterjamesnugent and will presumably also need updating - so would like to avoid making the change again if the decision is not concrete? Definitely BH.Engine.Adapters.Blah for anything we don't want exposed in UIs?

@pawelbaran
Copy link
Member

Hitting up @FraserGreenroyd's query - happy to action but could we please confirm we all are aligned?

@peterjamesnugent
Copy link
Member

@pawelbaran I've already made the changes on MidasCivil and Lusas.

@pawelbaran
Copy link
Member

Changes to External or Adapters?

@peterjamesnugent
Copy link
Member

Adapters

@al-fisher
Copy link
Member

Yes - can confirm Adapters is agreed namespace following all discussions

@alelom

@alelom
Copy link
Member Author

alelom commented May 21, 2020

Yes it is Adapters.

@alelom
Copy link
Member Author

alelom commented Feb 3, 2023

Closing as completed.

@alelom alelom closed this as completed Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:compliance Non-conforming to code guidelines
Projects
None yet
Development

No branches or pull requests

8 participants