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

Add acct_nt (T2 Lock) #463

Merged
merged 11 commits into from
May 17, 2024
Merged

Add acct_nt (T2 Lock) #463

merged 11 commits into from
May 17, 2024

Conversation

MichaelEpicA
Copy link
Contributor

Adds a full description and explanation of every mode and every way acct_nt can behave.

Problem

Fixes #461

Adds a full description and explanation of every mode and every way acct_nt can behave.
Copy link
Contributor

@Zachava96 Zachava96 left a comment

Choose a reason for hiding this comment

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

All articles should be written in third person with no referential nouns (e.g. you, your), except for Tutorials, FAQs, and Writeups.

acct_nt.mdx Outdated Show resolved Hide resolved
acct_nt.mdx Outdated Show resolved Hide resolved
acct_nt.mdx Outdated Show resolved Hide resolved
acct_nt.mdx Outdated Show resolved Hide resolved
acct_nt.mdx Outdated Show resolved Hide resolved
MichaelEpicA and others added 5 commits May 11, 2024 15:16
Fix you

Co-authored-by: Zachava96 <[email protected]>
Co-authored-by: Zachava96 <[email protected]>
Co-authored-by: Zachava96 <[email protected]>
Co-authored-by: Zachava96 <[email protected]>
Co-authored-by: Zachava96 <[email protected]>
Copy link
Contributor

@Zachava96 Zachava96 left a comment

Choose a reason for hiding this comment

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

Total spent/earned actually uses net, not just incoming or outgoing, my bad

acct_nt.mdx Outdated Show resolved Hide resolved
acct_nt.mdx Outdated Show resolved Hide resolved
@hmdunce
Copy link
Contributor

hmdunce commented May 16, 2024

Thanks both of you! I was going to see about merging but looks like there is still discussion.

I think it doesn't have to be perfect. Someone can always fix it later.

Copy link
Contributor

@hmdunce hmdunce left a comment

Choose a reason for hiding this comment

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

The file should be moved to docs/upgrades/locks so that it shows up in the locks list. Currently the page doesn't show up at all I think.

You can do it with

git mv acct_nt.mdx docs/upgrades/locks/

and then commit, push as usual.

I would just do this but I don't want to cause confusion for ongoing work on the file.

@CLAassistant
Copy link

CLAassistant commented May 16, 2024

CLA assistant check
All committers have signed the CLA.

@MichaelEpicA MichaelEpicA requested a review from hmdunce May 16, 2024 11:42
Copy link
Contributor

@hmdunce hmdunce left a comment

Choose a reason for hiding this comment

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

OK, I checked in codespaces and it looks good to me.

I am not checking for 100% accuracy since I don't think I've ever fully understood how acct_nt works, but I think that's fine.

I only have one question: do the stats and behavior sections contain spoilers? I've seen the README definition but I'm not sure exactly what it means. 😄

I don't know how much it matters though.

@MichaelEpicA
Copy link
Contributor Author

I don't think they do? It's pretty obvious that accts.transactions is nesseceary but if not, I can change that.

@hmdunce
Copy link
Contributor

hmdunce commented May 16, 2024

The meaning of acct_nt_min in particular does not seem obvious to me. I suspect what is written here may not even be correct??? 🤷‍♂️

@MichaelEpicA
Copy link
Contributor Author

acct_nt_min as close to my understanding is correct, since acct_nt uses a range of transactions and acct_nt_min is how small that range can be

@hmdunce
Copy link
Contributor

hmdunce commented May 16, 2024

Just to be safe, I will add a spoiler commit, you can tell me if it looks good to you?

By the way, just curious, are you able to push the "Squash and merge" button when it turns green? I don't know who has permission. If you are able, you should feel free to push it, IMO.

@MichaelEpicA
Copy link
Contributor Author

I do not have write access so no

@hmdunce
Copy link
Contributor

hmdunce commented May 16, 2024

Ok, it was probably unnecessary, but added some spoiler guards. If it looks good to you I will merge?

Thanks for your contribution!

@MichaelEpicA
Copy link
Contributor Author

Yep

@hmdunce hmdunce merged commit 0ef6ad6 into comcode-org:main May 17, 2024
4 checks passed
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.

acct_nt is not on the wiki
4 participants