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.

Sign up for GitHub

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

Jump to bottom

flat features #3685

Merged
merged 5 commits into from
Nov 9, 2019
Merged

flat features #3685

merged 5 commits into from
Nov 9, 2019

Conversation

cfromknecht
Copy link
Contributor

An implementation of lightning/bolts#666

@cfromknecht cfromknecht mentioned this pull request Nov 7, 2019
Merged
@cfromknecht cfromknecht force-pushed the flat-features branch 2 times, most recently from aec5dae to 3e64047 Compare November 8, 2019 02:51
@cfromknecht cfromknecht marked this pull request as ready for review November 8, 2019 02:51
@cfromknecht cfromknecht added this to the 0.9.0 milestone Nov 8, 2019
joostjager
joostjager reviewed Nov 8, 2019
View reviewed changes
Copy link
Contributor

@joostjager joostjager left a comment

Choose a reason for hiding this comment

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

First pass looking good. Main comment is about order of commits

channeldb/migration_01_to_11/graph.go Outdated Show resolved Hide resolved
peer.go Outdated Show resolved Hide resolved
feature/manager_test.go Outdated Show resolved Hide resolved
feature/default_sets.go Outdated Show resolved Hide resolved
@cfromknecht cfromknecht force-pushed the flat-features branch 4 times, most recently from 77cea27 to 9f80240 Compare November 8, 2019 10:36
joostjager
joostjager approved these changes Nov 8, 2019
View reviewed changes
Copy link
Contributor

@joostjager joostjager left a comment

Choose a reason for hiding this comment

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

Nice restructuring

lnwire/init_message.go Outdated Show resolved Hide resolved
@cfromknecht cfromknecht force-pushed the flat-features branch 4 times, most recently from b13cb6d to 79a0753 Compare November 8, 2019 11:45
halseth
halseth suggested changes Nov 8, 2019
View reviewed changes
lnwire/features.go Show resolved Hide resolved
feature/manager.go Show resolved Hide resolved
lnwire/features.go Outdated Show resolved Hide resolved
lnwire/features.go Outdated Show resolved Hide resolved
feature/manager.go Show resolved Hide resolved
server.go Show resolved Hide resolved
lnpeer/peer.go Show resolved Hide resolved
lnpeer/peer.go Outdated Show resolved Hide resolved
peer.go Outdated Show resolved Hide resolved
peer.go Show resolved Hide resolved
@cfromknecht
lnwire/features: add unified Features namespace
90e36ca
@cfromknecht
feature: add new feature pkg to manage feature sets
fe566e1 
This commit introduces a feature.Manager, which derives feature vectors
for various contexts within the daemon. The sets can be described via a
staticly compiled format, which makes any runtime adjustments to the
feature sets when the manager is initialized.
@cfromknecht cfromknecht mentioned this pull request Nov 8, 2019
Merged
halseth
halseth approved these changes Nov 8, 2019
View reviewed changes
Copy link
Contributor

@halseth halseth left a comment

Choose a reason for hiding this comment

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

LGTM 💯

Roasbeef
Roasbeef approved these changes Nov 9, 2019
View reviewed changes
Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

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

Clean set of changes! We should wait on merge until we confirm proper interop tests w/ the other main implementations, and also for the final version of the spec to be merged.

LGTM 💥

@joostjager joostjager merged commit b222b6e into lightningnetwork:master Nov 9, 2019
@halseth halseth mentioned this pull request Nov 11, 2019
Merged
@cfromknecht cfromknecht mentioned this pull request Dec 13, 2019
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joostjager joostjager joostjager approved these changes

@Roasbeef Roasbeef Roasbeef approved these changes

@halseth halseth halseth approved these changes

Assignees
No one assigned
Labels
amp spec
Projects
None yet
Milestone
0.9.0
Development

Successfully merging this pull request may close these issues.
4 participants
@cfromknecht @Roasbeef @halseth @joostjager