Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Oct 19, 2020

No description provided.

@alexis- alexis- self-requested a review November 5, 2020 12:38
Copy link
Member

@alexis- alexis- left a comment

Choose a reason for hiding this comment

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

  1. Can you fix the few issues I've outlined ?

  2. More globally, there are problems with the coding style (e.g. SMNatives.TContents, ContentsWdw, ...).

The easiest way to address this would be for you to install Resharper. It will load the project's coding style and automatically apply them. You can also run Resharper's "Refactor" function on existing code to reformat it (e.g. TNatives.TContents).

  1. For the rest, good work !

Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SuperMemoAssistant.VersionBuilder", "src\Core\SuperMemoAssistant.VersionBuilder\SuperMemoAssistant.VersionBuilder.csproj", "{712820C7-6A72-4829-8D39-EDE2FDB32666}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SuperMemoAssistant.Plugins.OmniMemo", "src\Plugins\SuperMemoAssistant.Plugins.OmniMemo\src\SuperMemoAssistant.Plugins.OmniMemo\SuperMemoAssistant.Plugins.OmniMemo.csproj", "{F737AECE-9DAB-40EB-A2EF-DE97F43BA87C}"
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SuperMemoAssistant.Plugins.TestSetConceptNo", "..\SuperMemoAssistant.Plugins.TestSetConceptNo\src\SuperMemoAssistant.Plugins.TestSetConceptNo\SuperMemoAssistant.Plugins.TestSetConceptNo.csproj", "{BAC96BE4-668C-477C-B02B-C496147EEC3E}"
Copy link
Member

Choose a reason for hiding this comment

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

Can you remove that Plugin ?

"ElWdw_ForceRepetitionExt": "55 8B EC 81 C4 ? ? ? ? 88 4D F7 89 55 F8 89 45 FC 8D 4D 81",
// ???? TElWind::MIForgetClick or TElWind::MIJumpIntervalClick => Last call
"ElWdw_RestoreLearningMode": "55 8B EC 51 89 45 FC 8B 45 FC 80 B8 ? ? ? ? ? 75 1E"
"ElWdw_RestoreLearningMode": "55 8B EC 51 89 45 FC 8B 45 FC 80 B8 ? ? ? ? ? 75 1E",
Copy link
Member

Choose a reason for hiding this comment

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

There shouldn't be a comma at the end of the line

Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SuperMemoAssistant.VersionBuilder", "src\Core\SuperMemoAssistant.VersionBuilder\SuperMemoAssistant.VersionBuilder.csproj", "{712820C7-6A72-4829-8D39-EDE2FDB32666}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SuperMemoAssistant.Plugins.OmniMemo", "src\Plugins\SuperMemoAssistant.Plugins.OmniMemo\src\SuperMemoAssistant.Plugins.OmniMemo\SuperMemoAssistant.Plugins.OmniMemo.csproj", "{F737AECE-9DAB-40EB-A2EF-DE97F43BA87C}"
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SuperMemoAssistant.Plugins.TestSetConceptNo", "..\SuperMemoAssistant.Plugins.TestSetConceptNo\src\SuperMemoAssistant.Plugins.TestSetConceptNo\SuperMemoAssistant.Plugins.TestSetConceptNo.csproj", "{BAC96BE4-668C-477C-B02B-C496147EEC3E}"
Copy link
Member

Choose a reason for hiding this comment

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

You'll need to pull the latest version, the OmniMemo plugin is being erased


if (builder.Concept != null)
if (Core.SM.UI.ElementWdw.SetCurrentConcept(builder.Concept.Id) == false)
if (Core.SM.UI.ContentWdw.MoveElementToConcept(this[elemId].Id,builder.Concept.Id) == false)
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a space after the comma ?

if (Core.SM.UI.ContentWdw.MoveElementToConcept(this[elemId].Id, builder.Concept.Id)

public TRegistry Registry { get; }
public TFileSpace FileSpace { get; }
public TQueue Queue { get; }
public TContents Contents { get; }
Copy link
Member

Choose a reason for hiding this comment

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

Missing a space for alignment (coding style)


public ContentWdw()
{
Core.SMA.OnSMStartedEvent += OnSMStartedEventAsync;
Copy link
Member

Choose a reason for hiding this comment

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

Can you replace with OnSMStarting ?



#region Methods
private async Task OnSMStartedEventAsync(object sender,
Copy link
Member

Choose a reason for hiding this comment

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

Can you replace with OnSMStarting ?

</ItemGroup>
<ItemGroup>
<PackageReference Update="Nerdbank.GitVersioning" Version="3.2.31" />
</ItemGroup>
Copy link
Member

Choose a reason for hiding this comment

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

Why was that added ?

Copy link
Author

Choose a reason for hiding this comment

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

I have no clue how but will remove it.

@alexis- alexis- added enhancement New feature or request sma-api SMA interface API labels Nov 5, 2020
@alexis- alexis- added this to the SMA 2.1.0 "Open-Beta" milestone Nov 5, 2020
@alexis- alexis- assigned ghost Nov 5, 2020
@ghost ghost requested a review from alexis- November 9, 2020 16:34
Copy link
Member

@alexis- alexis- left a comment

Choose a reason for hiding this comment

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

All good except the versions.json. When you commit make sure to compare all the files one by one.

I've updated them and I'm accepting the PR thanks !

@alexis- alexis- self-requested a review November 10, 2020 17:41
Copy link
Member

@alexis- alexis- left a comment

Choose a reason for hiding this comment

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

I actually missed something during my review. The .sln seems a little messed up. At a very quick glance I identified:

  • You've added a x64 platform
  • Some configurations are missing (NuPack)

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request sma-api SMA interface API

Development

Successfully merging this pull request may close these issues.

2 participants