Skip to content

Improve the naming conventions of classes and methods in C# bindings#1

Closed
Mersho wants to merge 3 commits intopackwallet:mainfrom
Mersho:ImproveLDK-Squashed
Closed

Improve the naming conventions of classes and methods in C# bindings#1
Mersho wants to merge 3 commits intopackwallet:mainfrom
Mersho:ImproveLDK-Squashed

Conversation

@Mersho
Copy link

@Mersho Mersho commented Mar 11, 2024

Some jobs have been removed due to their failure in
the upstream repository.
@Mersho Mersho force-pushed the ImproveLDK-Squashed branch from 4a6f51b to 451bccc Compare March 11, 2024 10:56
We change all interfaces to PascalCase in C# to comply with
the C# coding style and the C# code conventions as outlined
by Microsoft [1]:

- "Interface names start with a capital I."

[1] https://learn.microsoft.com/en-us/dotnet/csharp/fundamentals/coding-style/identifier-names
@Mersho Mersho force-pushed the ImproveLDK-Squashed branch from f5fa245 to 451bccc Compare March 11, 2024 11:00
@aarani
Copy link

aarani commented Mar 11, 2024

I guess function names like "decodeUint8Array" needs to be updated as well

@siwatanejo
Copy link
Member

I guess function names like "decodeUint8Array" needs to be updated as well

Agreed.

We change all method and class names to PascalCase in C# to
comply with the C# coding style and the C# code conventions as
outlined by Microsoft [1]:

- "Use PascalCase for class names and method names."

[1] https://learn.microsoft.com/en-us/dotnet/csharp/fundamentals/coding-style/identifier-names
@Mersho Mersho force-pushed the ImproveLDK-Squashed branch from 03a45ba to c27c3b4 Compare March 11, 2024 11:29
@siwatanejo
Copy link
Member

CI is red, why was a PR proposed if CI is still red?

@Mersho
Copy link
Author

Mersho commented Mar 11, 2024

CI is red, why was a PR proposed if CI is still red?

The c_sharp_bindings job is fine, but I'm not understanding why macos_determinism is failing. However, I'm going to investigate it.

@siwatanejo
Copy link
Member

Just looked at the logs and my suspicion is that a re-run will work (of course I'm talking only about the CI of the 1st commit).

@Mersho
Copy link
Author

Mersho commented Mar 11, 2024

After comparing the logs with upstream I realized the problem:

It's related to git describe --tag HEAD. In the CI, a curl SNAPSHOT_LINK command is executed based on git describe --tag HEAD

There's two solutions:

  1. Removing this job like other jobs
  2. Use git describe --tag --abbrev=0 instead of git describe --tag HEAD (I'm not 100% sure about this solution)

If you approve and let me: I also going to remove that job.

@aarani
Copy link

aarani commented Mar 11, 2024

This project needs to be on a tag, as the error message clearly says.

@Mersho
Copy link
Author

Mersho commented Mar 12, 2024

This project needs to be on a tag, as the error message clearly says.

You're looking at the wrong place (PR CI)...
let me explain git describe --tag HEAD output:

v0.0.121.2-3-gc27c3b4f:

v0.0.121.2: The most recent reachable tag is v0.0.121.2.
3: There are 3 additional commits after the v0.0.121.2 tag.
gc27c3b4f: The g stands for "git", and c27c3b4 is the abbreviated commit hash of HEAD.

@siwatanejo
Copy link
Member

Superseded by #4

@siwatanejo siwatanejo closed this Mar 16, 2024
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