Skip to content

An attempt at building a cross platform executable with dotnet #12

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

rockdreamer
Copy link
Contributor

No description provided.

# Local History for Visual Studio
.localhistory/

# BeatPulse healthcheck temp database

Choose a reason for hiding this comment

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

This looks irrelevant, as do quite a few entries in this file. It's difficult to see what is relevant.

<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net5.0</TargetFramework>

Choose a reason for hiding this comment

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

I wondered if it would be better to use Net Core rather than the full framework?

// run CommitHookCheckInterface checks
// return 0 if everything is ok
// or 1 if something should be aborted
return 0;

Choose a reason for hiding this comment

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

This doesn't appear to do anything - ah - now I realise this is just a framework, not a complete implementation.


This is currently used to decide if large file should be commited.

Choose a reason for hiding this comment

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

These changes look sensible, but I feel they should be removed from this PR as it makes it impossible to merge it to main as it replaces the python calls but without replacing them with a working C# version. I feel it would be better to be able to merge the C# to main independently of these changes to make them the default.

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

Successfully merging this pull request may close these issues.

2 participants