Rad registry #652

MeBrei · 2019-05-31T10:13:57Z

A registered project has name, description, labels & project-id.
Allowed commands are registering, editing, listing (and adding admins)

TODO change default machine name to server hosted one

jkarni · 2019-06-05T08:54:47Z

bin/rad-registry

+
+;; TODO Replace with server hosted machine
+(def default-machine-name
+  "The default registry machine."


Why a function?

It doesn't have to be a function. Changing it.

jkarni · 2019-06-05T08:56:35Z

bin/rad-registry

+      :nothing    (default-machine-name)
+      [:just 'r]  r)))
+
+(def valid-states ["open" "closed"])


Is there a reason to keep this as state? Why not just delete projects?

Will remove the state add commands for deleting projects.

jkarni · 2019-06-05T08:57:10Z

bin/rad-registry

+
+(def valid-states ["open" "closed"])
+
+(def status-opts ["--filter-by-state" "--state" "-s"])


Related to the above, I don't really see the point in being able to filter. If you removed a project, you shouldn't be able to see it at all.

jkarni · 2019-06-05T08:57:32Z

bin/rad-registry

+   Usage:
+        rad registry list " list-opts-help "
+        rad registry new
+        rad registry [show | close | mark-read | mark-unread] <project-number>


Rather than close, I'd call it remove or delete.

jkarni · 2019-06-05T09:06:57Z

bin/rad-registry

+    (catch 'daemon-error
+      (do
+        (items/verify-item-number n (list-projects machine) :project)
+        (simple-edit-project! machine n {:state :closed})


You seem to use this three times. Maybe make a helper function?

jkarni · 2019-06-05T09:24:28Z

rad/prelude/error-messages.rad

@@ -82,11 +82,11 @@

 (def uppercase-item
  (fn [item]
-    (lookup item {:issue "Issue" :patch  "Patch"})))
+    (lookup item {:issue "Issue" :patch "Patch" :project "Project"})))


(def uppercase-item [item] (titlecase (lowercase-item item))

You'll need to add some sort of pattern matching or similar functionality to implement titlecase

(See above) The flag and item name does not match in this way anymore.

jkarni · 2019-06-05T09:24:52Z

rad/prelude/io.rad

@@ -260,6 +260,16 @@ process exited normally and `[:error n]` otherwise. Example: `(process!
    (write-handle! (stderr!) prompt)
    (read-line-handle! (stdin!))))

+(def-rec prompt-non-empty!
+  "Same as `prompt!`, but repeats id the input empty."


Suggested change

"Same as `prompt!`, but repeats id the input empty."

"Same as `prompt!`, but repeats if the input is empty."

jkarni · 2019-06-05T09:26:30Z

rad/prelude/io.rad

@@ -260,6 +260,16 @@ process exited normally and `[:error n]` otherwise. Example: `(process!
    (write-handle! (stderr!) prompt)
    (read-line-handle! (stdin!))))

+(def-rec prompt-non-empty!


This doesn't seem to be a very general function. Ideally the abstraction is something that takes a validator, an error message, and repeats the prompt until the validator passes.

jkarni · 2019-06-05T09:27:50Z

rad/monadic/registry-remote.rad

+    validator/time-created
+    validator/full-project]))
+
+(def edit-keys


Can you explain what this does, or have a more descriptive name?

jkarni · 2019-06-05T09:29:49Z

rad/monadic/items.rad

@@ -118,6 +146,24 @@
        "Comments\n"
        "--------\n\n"
        (pretty-comments cs)))
+      '{:number n


It's very weird to me to have a single function that is supposed to know how to render every different item comment type. Why not different functions?

I agree that items.rad needs some refactoring. In this case it is convenient to have the function pretty-item-view handle distinguishing between the item types, otherwise there would be more matching where the function is used.
Did you have an idea of how to split it into different functions? I can only think of an option to pass flags to the functions of items and depending on that use specific functions, but I am not sure that will improve things.

I agree that flags are also not good (even in pretty-table-header it looks bad).
I don't entirely understand the "more matching" problem. There'd be a function, pretty-issue, that would call pretty-issue-header, and so on. You don't need to match on the comment (unless you want to) if you already know at call-site what shape it'll have (which you will since you'll be calling from inside a function that knows the comment shape).

But then you would still have to decide what function to call depending on the flag. So for example in list-items for each of the following function you would have to match/decide which specific one to call: pretty-headline, pretty-row, pretty-table-header. So would you have them refer to the specific functions depending on the flag?

So for example in list-items for each of the following function you would have to match/decide which specific one to call: pretty-headline, pretty-row, pretty-table-header

I don't understand this. Can you explain?

Here's the rule. If I define a new machine, with it's own set of items, and I don't have write access to this repo, can I re-use any of the functionality? Clearly, right now the answer is "no". That means the abstractions are wrong. Maybe there is no common abstraction, but probably the answer is more of a middle ground: we can't have "do everything" functions such as pretty-item-view, but we can have other functions that respect actual invariants. E.g.:

(def prompt-comment (fn [post] ...))

Which takes some string post to put below the ---, puts that and the line in a file, and sets you up in an editor, and gives you back the string that was inputed.

I agree with you. I think I am just not fully clear on which functions to abstract and which not, yet (fe list-items) I think the general direction is clear, but I think it exceeds this PR as it would also change issue and patch code. I can write create an issue for the refactoring.

ok (though in general I'm in favor of the refactor-what-you-touch philosophy)

Refactoring issue: #665

For single-constructor Haskell data types: - `toRad` will skip the constructor, - `fromRad` will optionally parse the constructor. BREAKING CHANGE: This changes the format for key-pairs and signatures.

Transactor re-definition replaces eval re-difinition. This is a much simpler model. At first tx is set to the identity function, that is, the machine accepts all input expressions as is, and evaluates them. One can then redefine tx, to restrict the inputs to the machine. If tx throws an exception, then the input is not accepted, otherwise it can perform effects itself (e.g. mutate some refs), and then return an expression for evaluation on the machine. See https://github.com/radicle-dev/transparent-machines-doc for more information. BREAKING CHANGE: This totally changes how machines restrict their inputs, so is a major breaking change.

Non-hygienic first-class macros whose expanded code evaluates in a scope. BREAKING CHANGE: in particular changes `if` is now not a special-form and `test` is a macro which replaces `(:test ...)`.

BREAKING CHANGE: This changes the structure of the message that is signed before sending it to a machine. Machines set up before this commit will not accept the new signatures. `show-unbound` does not add formatting to the requests payload, to make signing request consistently to work also in other languages.

jkarni · 2019-06-06T13:38:14Z

rad/prelude/io.rad

@@ -260,6 +260,18 @@ process exited normally and `[:error n]` otherwise. Example: `(process!
    (write-handle! (stderr!) prompt)
    (read-line-handle! (stdin!))))

+(def-rec prompt-non-empty!


The name is now outdated.

Yes, sorry for that!

jkarni · 2019-06-06T13:39:37Z

rad/prelude/io.rad

+  input does not pass the `validator`."
+  (fn [prompt validator err-msg]
+    (write-handle! (stderr!) prompt)
+    (def input (read-line-handle! (stdin!)))


why not just use prompt!?

MeBrei changed the base branch from master to v2 May 31, 2019 10:14

MeBrei force-pushed the v2 branch from 93629e5 to 4b59b34 Compare June 4, 2019 13:00

MeBrei force-pushed the f/registry branch from b01706d to 37d60ca Compare June 5, 2019 07:29

MeBrei marked this pull request as ready for review June 5, 2019 08:26

MeBrei requested review from jameshaydon and jkarni June 5, 2019 08:38

jkarni reviewed Jun 5, 2019

View reviewed changes

jameshaydon force-pushed the v2 branch from 4b59b34 to b0d3826 Compare June 5, 2019 09:43

jameshaydon and others added 13 commits June 5, 2019 15:25

feat: Unwrap Generic FromToRad for single constructor datatypes (#583)

26384f4

For single-constructor Haskell data types: - `toRad` will skip the constructor, - `fromRad` will optionally parse the constructor. BREAKING CHANGE: This changes the format for key-pairs and signatures.

feat: Macros (#647)

33f5ba5

Non-hygienic first-class macros whose expanded code evaluates in a scope. BREAKING CHANGE: in particular changes `if` is now not a special-form and `test` is a macro which replaces `(:test ...)`.

WIP registry remote

be0c4b5

Add rad registry commands

e49d503

Extend error messages with project tag

7089611

Set registry machine

4ca2f5e

Add default machine and non empty prompt

3205e65

Add reference doc

f9b1e6f

Address some of the cr comments

c0853cc

WIP address more cr comments

9cb697a

Rename registry items to registered-machine

849899e

MeBrei force-pushed the f/registry branch from e5794a0 to 849899e Compare June 6, 2019 12:37

MeBrei changed the base branch from v2 to v2.0.0 June 6, 2019 12:37

Use short lambda instead of merge-new

d5f4ff1

jkarni reviewed Jun 6, 2019

View reviewed changes

Merle Breitkreuz added 2 commits June 6, 2019 16:10

Rename prompt-non-empty! to prompt-validated!

739e735

Improve indentation of list-items

fe520a8

jameshaydon force-pushed the v2.0.0 branch from cf3ebd2 to 887dca2 Compare July 11, 2019 09:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rad registry #652

Rad registry #652

MeBrei commented May 31, 2019 •

edited

Loading

jkarni Jun 5, 2019

MeBrei Jun 5, 2019

jkarni Jun 5, 2019

MeBrei Jun 5, 2019

jkarni Jun 5, 2019

jkarni Jun 5, 2019

jkarni Jun 5, 2019

jkarni Jun 5, 2019

MeBrei Jun 6, 2019

jkarni Jun 5, 2019

jkarni Jun 5, 2019

jkarni Jun 5, 2019

jkarni Jun 5, 2019

MeBrei Jun 5, 2019

jkarni Jun 6, 2019

MeBrei Jun 6, 2019

jkarni Jun 6, 2019

MeBrei Jun 6, 2019

jkarni Jun 6, 2019 •

edited

Loading

MeBrei Jun 6, 2019

jkarni Jun 6, 2019

MeBrei Jun 6, 2019

jkarni Jun 6, 2019


		(def valid-states ["open" "closed"])

		(def status-opts ["--filter-by-state" "--state" "-s"])

	"Same as `prompt!`, but repeats id the input empty."
	"Same as `prompt!`, but repeats if the input is empty."

Rad registry #652

Are you sure you want to change the base?

Rad registry #652

Conversation

MeBrei commented May 31, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkarni Jun 6, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MeBrei commented May 31, 2019 •

edited

Loading

jkarni Jun 6, 2019 •

edited

Loading