Proposal: the string interpolation (the template string)

Instructions: Replace the template text and remove irrelevant text (including this line)

Hi, Bram.
I'm a you and Vim's fan!
Is your feature request related something that is currently hard to do? Please describe.
I want to introduce the string interpolation (the template string) into Vim script.
Now it is introduced onto

JavaScript

and TypeScript

Kotlin
Scala
Ruby
C#
Swift

I never contributed Vim yet, but this is not so hard in my opinion...
Describe the solution you'd like
A clear and concise description of what you want to happen.
'The string interpolation' ('The template string') embeds any expressions ${x} into a string via Vim function string().
For example.
" Embed literals using back quotations (the template string) and "${" "}"
call assert_equal(`I have ${10}`, 'I have 10')
call assert_equal(`I'm a ${'vim'}`, "I'm a vim")

" Variables
let x = 10
call assert_equal(`I have ${x}`, 'I have 10')

" Others
let x = ch_open('localhost:25252')  " dummy
call assert_equal(`${x}`, string(x))

I'm refering to the JavaScript's format of ${x} for the Vim script's template string.
(${x} ${dict.a} ${10} ${function('function')})
These expressions must be surrounded by "${" and "}".

I'm implementing it now at this branch.
The test can be shown at here :point_down:

vim/test_string_interpolation.vim at ec933cee9d9cb2b1bd5b3f3fa69f8dc214989c2a - aiya000/vim - GitHub

Please see this :point_up: if you can!
Describe alternatives you've considered

A clear and concise description of any alternative solutions or features you've considered.

I'm planning a template string form $"I'm ${name}" (surrounded by $" and ")
if Bram doesn't like a back quoted form I'm ${name}.
Additional context
After it introduced, expression embedding is to be more easier, and I think it is very cool!

Thanks!

I'm implementing this at here :D

Backticks are hard to use on some keyboards, and also they are hard to distinguish from single quotes.
On top of that, Vim has two types of strings: single quoted and double quoted.  Both are useful.
Therefore the $"template" and $'template' syntax seems a better solution.
Since we only used $ for environment variables, I don't think there is a conflict with existing syntax.
For $"foo ${var} bar" I assume that "var" is taken literally, thus backslashes are kept as-is.

Thank you, Bram!
That solution seems very good!
I'll implement $'' and $"" :+1:

I force-pushed the newer test here :)

What about instead adding a function similar to eval() or map() to perform the interpolation?  Eg interpol().
Or what about instead adding a function to create an intermediate representation (ie a Vim expression) which can then be eval()uated?
And for the syntax:
We already have statusline syntax:  abc%{expr}def, how about reusing that?

I force-pushed the newer test here :)

Thanks.  I would think this works fine.
I have some minor problem with using $"" and $'', since it results in a
lot of dollars, e.g.:
$'${[10, 20]}'

Is there any other syntax that would work better?
t'${[10, 20]}'
_'${[10, 20]}'

Opinions?
Should we also allow using $var instead of ${var}, like Python does?
It does require escaping every literal $ with $$.
--
hundred-and-one symptoms of being an internet addict:
218. Your spouse hands you a gift wrapped magnet with your PC's name
on it and you accuse him or her of genocide.
/// Bram Moolenaar --<EMAIL_ADDRESS>-- http://www.Moolenaar.net   \
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \
\\  an exciting new programming language -- http://www.Zimbu.org        ///
\\            help me help AIDS victims -- http://ICCF-Holland.org    ///

What about instead adding a function similar to eval() or map()
to perform the interpolation?  Eg interpol().
Or what about instead adding a function to create an intermediate
representation (ie a Vim expression) which can then be eval()uated?

Python has the Template() constructor for this.  It's a more heavy
mechanism.

And for the syntax: We already have statusline syntax:
abc%{expr}def, how about reusing that?

That makes sense.  Javascript, Typescript and Python use ${expr} though.
Also, printf() templates use %.  Not sure what weighs more.
--
hundred-and-one symptoms of being an internet addict:
219. Your spouse has his or her lawyer deliver the divorce papers...
via e-mail.
/// Bram Moolenaar --<EMAIL_ADDRESS>-- http://www.Moolenaar.net   \
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \
\\  an exciting new programming language -- http://www.Zimbu.org        ///
\\            help me help AIDS victims -- http://ICCF-Holland.org    ///

I have some minor problem with using $"" and $'', since it results in a
lot of dollars, e.g.:
Is there any other syntax that would work better?
Opinions?

I like $"" and $'' the best, but I think it is good to add 't'' and t""' or '_'' and _""' as that synonym :+1:

Should we also allow using $var instead of ${var}, like Python does?

I thought it, and took a questionnaire to
What do you like any support?

1. only ${var}
2. only $var
3. both ${var} and $var

the 1. is gotten most votes!
I think $var can be appended later if $var is needed later.
But I wish to will start off of with only ${var} now :joy:
(I don't hate $var :D)

It does require escaping every literal $ with $$.

Good!
I'll implement the escaping as it!

The 3 types of string interpolation I'm used to seeing and using are:

javascript/typescript:`Value is: ${val}`

python:f'Value is {val}'
f'Literal braces: {{}}'

C#:$"Value is {val}"
$"Literal braces: {{}}"

Keeping similar to at least one of these seems sensible - since javascript is so popular these days my preference would be to use that syntax. The other nice thing about the javascript/typescript syntax is that you don't have to escape literal characters - $, { and } can all be used alone, the interpolation only captures the full ${} pattern.

I don't see the point of any of this, as opposed to just using existing concatenation.  Doing let foo = "one $[bar} baz" saves only one measly character when compared with let foo = "one ".bar." baz"

@chdiza Maybe it is a question for general :joy:
Please find the purpose of other languages :)

The 3 types of string interpolation I'm used to seeing and using are:

javascript/typescript:`Value is: ${val}`

python:f'Value is {val}'
f'Literal braces: {{}}'

C#:$"Value is {val}"
$"Literal braces: {{}}"

Keeping similar to at least one of these seems sensible - since
javascript is so popular these days my preference would be to use that
syntax. The other nice thing about the javascript/typescript syntax is
that you don't have to escape literal characters - $, { and }
can all be used alone, the interpolation only captures the full ${}
pattern.

I also use Javascript/Typescript regularly, which is why I know the
backtick is hard to recognize.  It depends on the font, but it looks
very much like the backtick.  This may lead to mistakes.
I agree that using just $val or {val} has the problem of much more
escaping.  "${" is pretty rare to encounter.
I actually like how _"template" and _'template' look, but I don't think
any other language does that, thus it's a bit weird.
--
hundred-and-one symptoms of being an internet addict:
234. You started college as a chemistry major, and walk out four years
later as an Internet provider.
/// Bram Moolenaar --<EMAIL_ADDRESS>-- http://www.Moolenaar.net   \
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \
\\  an exciting new programming language -- http://www.Zimbu.org        ///
\\            help me help AIDS victims -- http://ICCF-Holland.org    ///

After all, I like $var too :D
I want to implement

$"" and _""
$'' and _''
$var ${var} ${literals}

var doesn't need the surround {}, literals must be surrounded by {}. e.g. $10 and $string(30) is illformed. $var and ${var} is well-formed.

Can I make this? :smiley:

(I like a form $'' and $"" :joy: I want to this.)

Isn't $var confusing with environment variables?

I also use Javascript/Typescript regularly, which is why I know the
backtick is hard to recognize.  It depends on the font, but it looks
very much like the backtick.  This may lead to mistakes.

OK, that's fair enough Bram (although I don't personally find it confusing and, as noted, this is already used by javascript/typescript so clearly many people find it acceptable).
My one last statement in favour of the javascript/typescript syntax is that vim already uses the {} interpolation characters in :h curly_brace_names, where it is possible in vimscript to do this:
let my_{&filetype}_variable = 'something';

It would be nice and consistent to then be able to do this:
let my_{&filetype}_variable = `something {&filetype}-specific`;

Apologies, I'm clearly typing before thinking, the curly_brace_name syntax is similar to C# syntax, not javascript/typescript syntax, as there is no leading $ before the interpolation {} characters. So perhaps the C# syntax would be appropriate?
let my_{&filetype}_variable = $"something {&filetype}-specific (escaping with doubled characters for literal {{ and }})"

Just idea, but from Swift langauge's string literal, I wonder \(...) with extending existing double quote strings might match to Vim script.
print("6 times 7 is \(6 * 7).")

Though it does not fully maintain backward compatibility, \( simply represents( in "..." literal so it should not appear in current Vim script codebases.

Pros: we don't need a new type of string literal
Cons: backward compatibility is not fully maintained

Just idea, but from Swift langauge's string literal, I wonder (...) with extending existing double quote strings might match to Vim script.
print("6 times 7 is (6 * 7).")

Oh, I didn't bethink "(xxx)"..., but this is the best choice in my opinion!
I'll re-implementing all to this if Bram is OK :)
(
Does this mean that doesn't support string-interpolations in string literals?
echo "\(10)"
" ==> 10

echo '\(10)'
" ==> \(10)

I think this is very good.
)

Thank you for the quick and positive response.

Does this mean that doesn't support string-interpolations in string literals?

Yes, I meant that. Since single quote string is something like raw string literals in other languages, no interpolation sounds natural for me.
But it's just an idea. There may be some mismatch to Vim script. So let's carefully consider grammar. After some thoughts, I came up with possible confusion. \(...) in double quote is similar to \(...\) in single quote string (grouping of regex).

I also use Javascript/Typescript regularly, which is why I know the
backtick is hard to recognize.  It depends on the font, but it looks
very much like the backtick.  This may lead to mistakes.

I of course meant that a backtick looks very much like a single quote.

OK, that's fair enough Bram (although I don't personally find it
confusing and, as noted, this is already used by
javascript/typescript so clearly many people find it acceptable).

These decisions are often made by a committee, it doesn't mean it's
always a good idea.  Perhaps they used a font where it is not a problem.
For Vim it's nice if we can use a syntax that script writers recognize,
but we also want to avoid things that have turned out to be a
not-so-great choice.
There are other languages where a prefix to a string changes the meaning
of that string, we can at least follow that pattern.

My one last statement in favour of the javascript/typescript syntax is
that vim already uses the {} interpolation characters in :h curly_brace_names, where it is possible in vimscript to do this:
let my_{&filetype}_variable = 'something';

It would be nice and consistent to then be able to do this:
let my_{&filetype}_variable = `something {&filetype}-specific`;

The use of {} like that has been deprecated, there are too many subtle
corner cases with it. I also don't see it used much.
--
"The amigos also appear to be guilty of not citing the work of others who had
gone before them.  Even worse, they have a chapter about modeling time and
space without making a single reference to Star Trek!"
(Scott Ambler, reviewing the UML User Guide)
/// Bram Moolenaar --<EMAIL_ADDRESS>-- http://www.Moolenaar.net   \
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \
\\  an exciting new programming language -- http://www.Zimbu.org        ///
\\            help me help AIDS victims -- http://ICCF-Holland.org    ///

Just idea, but from Swift langauge's string literal, I wonder \(...) with extending existing double quote strings might match to Vim script.
print("6 times 7 is \(6 * 7).")

Though it does not fully maintain backward compatibility, \( simply represents( in "..." literal so it should not appear in current Vim script codebases.

Pros: we don't need a new type of string literal
Cons: backward compatibility is not fully maintained

I do not see a good reason to break backwards compatibility.
Also, for single quotes a backclash is not a special character.
I would not want to limit this to double quoted strings.
--
hundred-and-one symptoms of being an internet addict:
239. You think "surfing" is something you do on dry land.
/// Bram Moolenaar --<EMAIL_ADDRESS>-- http://www.Moolenaar.net   \
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \
\\  an exciting new programming language -- http://www.Zimbu.org        ///
\\            help me help AIDS victims -- http://ICCF-Holland.org    ///

I would not want to limit this to double quoted strings.

I see :+1:
I'll proceed with $'' $"" and _'' _"".
Thanks!

Hi,
On Wed, Jun 19, 2019 at 6:47 AM Bram Moolenaar
<EMAIL_ADDRESS>wrote:

I also use Javascript/Typescript regularly, which is why I know the
backtick is hard to recognize. It depends on the font, but it looks
very much like the backtick. This may lead to mistakes.

I of course meant that a backtick looks very much like a single quote.

OK, that's fair enough Bram (although I don't personally find it
confusing and, as noted, this is already used by
javascript/typescript so clearly many people find it acceptable).

These decisions are often made by a committee, it doesn't mean it's
always a good idea. Perhaps they used a font where it is not a problem.
For Vim it's nice if we can use a syntax that script writers recognize,
but we also want to avoid things that have turned out to be a
not-so-great choice.
There are other languages where a prefix to a string changes the meaning
of that string, we can at least follow that pattern.

My one last statement in favour of the javascript/typescript syntax is
that vim already uses the {} interpolation characters in :h curly_brace_names, where it is possible in vimscript to do this:
let my_{&filetype}_variable = 'something';

It would be nice and consistent to then be able to do this:
let my_{&filetype}_variable = `something {&filetype}-specific`;

The use of {} like that has been deprecated, there are too many subtle
corner cases with it. I also don't see it used much.

The taglist plugin (which was developed back in 2002 and widely used)
heavily uses the curly brace syntax in variables. This was implemented
before Lists/Dicts were added to Vim.
https://www.vim.org/scripts/script.php?script_id=273
http://vim-taglist.sourceforge.net/index.html

Yegappan

Now implement state!

Maybe, most implementation is done
A little of parsing is rest
Currently using back quotations. $'' $"" and _'' _"" will be used

The taglist plugin (which was developed back in 2002 and widely used)
heavily uses the curly brace syntax in variables. This was implemented
before Lists/Dicts were added to Vim.

In my opinion, it has no problem :+1:
Because string interpolations is available only under the $'' $"" _'' _""!
Please see above comments for back compatibilities if you want :D

I'm working on this about everyday, please wait! :D

My all tests passed!
This is checked on this revision!!
Remain tasks

Add help
Make syntax highlighting

Make syntax highlighting

Syntax highlighting is maintained differently.
It is maintained by Dr. Chip, so you should sent a patch to him.

_"foo" breaks compatibility. Current behavior is:
:let _ = 1
:echo _"foo"
1 foo

@brammool
Hi, I opened #4634 !
That is '' omitted version. because https://github.com/vim/vim/issues/4491#issuecomment-509065896 X)
Please take this patch if you ok to agree '' unsupport :pray:
Or let's discuss again about '_' support if you really wan to it :+1:

@brammool
Hi, I opened #4634 !
That is '' omitted version. because https://github.com/vim/vim/issues/4491#issuecomment-509065896 X)
Please take this patch if you ok to agree '' unsupport :pray:
Or let's discuss again about '_' support if you really wan to it :+1:

Thanks!
I do like how _"some ${var} here" looks, but it is an unusual syntax.
If you use gettext in Vim you wonder if the parens are missing:
_("some text here");
Although $"some ${var} here" doesn't look as nice, I haven't seen a
better syntax.  Thus I think we should go with that.
--
ARTHUR:  Bloody peasant!
DENNIS:  Oh, what a give away.  Did you hear that, did you hear that, eh?
That's what I'm on about -- did you see him repressing me, you saw it
didn't you?
The Quest for the Holy Grail (Monty Python)
/// Bram Moolenaar --<EMAIL_ADDRESS>-- http://www.Moolenaar.net   \
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \
\\  an exciting new programming language -- http://www.Zimbu.org        ///
\\            help me help AIDS victims -- http://ICCF-Holland.org    ///

I wonder why two syntaxes of same feature are supported (_"some ${var} here", $"some ${var} here").
I don't disagree strongly that adding two syntaxes, but as my simple feeling, I think one syntax is enough.
IMHO, I recommend Python 3's f-strings syntax (because Vim script syntax is similar to Python):
https://docs.python.org/ja/3.6/reference/lexical_analysis.html#f-strings
$ python3.7
Python 3.7.3 (default, Mar 27 2019, 09:23:15)
[Clang 10.0.1 (clang-1<IP_ADDRESS>)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> var = 42
>>> print(f'some {var} here')
some 42 here
>>> print(f'some {{var}} here')
some {var} here

But f-strings has many many features (below example is from above link).
I think it's not necessary (inside {...} is just Vim expression).
>>> name = "Fred"
>>> f"He said his name is {name!r}."
"He said his name is 'Fred'."
>>> f"He said his name is {repr(name)}."  # repr() is equivalent to !r
"He said his name is 'Fred'."
>>> width = 10
>>> precision = 4
>>> value = decimal.Decimal("12.34567")
>>> f"result: {value:{width}.{precision}}"  # nested fields
'result:      12.35'
>>> today = datetime(year=2017, month=1, day=27)
>>> f"{today:%B %d, %Y}"  # using date format specifier
'January 27, 2017'
>>> number = 1024
>>> f"{number:#0x}"  # using integer format specifier
'0x400'

As @mattn said, f-strings syntax has also compatibility issue https://github.com/vim/vim/issues/4491#issuecomment-509065896
But I think this is very minor edge case (specific to :echo and its familiar commands).

I want $'' $"" if only one is supported :D

@brammool Thanks for agreeing!
I'll fix PR reviewed terms momently 👍

I fixed reviews!

I wonder why two syntaxes of same feature are supported (_"some ${var} here", $"some ${var} here").
I don't disagree strongly that adding two syntaxes, but as my simple
feeling, I think one syntax is enough.

The current plan is to only support $'string' and $"string".

IMHO, I recommend Python 3's f-strings syntax (because Vim script
syntax is similar to Python):
https://docs.python.org/ja/3.6/reference/lexical_analysis.html#f-strings

There are other languages with other solutions.  I don't think any of
them really stands out, it mostly depends on what you are used to.
I use Javascript/Typescript myself, and the backtick way is nicely
short, but I also have the experience that distinguising a single quote
from a backtick is quite hard, it's very easy to do wrong.  It depends
on the font too.
--
-rwxr-xr-x  1 root          24 Oct 29  1929 /bin/ed
-rwxr-xr-t  4 root      131720 Jan  1  1970 /usr/ucb/vi
-rwxr-xr-x  1 root  5.89824e37 Oct 22  1990 /usr/bin/emacs
/// Bram Moolenaar --<EMAIL_ADDRESS>-- http://www.Moolenaar.net   \
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \
\\  an exciting new programming language -- http://www.Zimbu.org        ///
\\            help me help AIDS victims -- http://ICCF-Holland.org    ///

#4639 finished!
I'll wait to be taken the patch :D
Thanks!

@aiya000 maybe the pull request should be #4634.
I also want this feature.
because it improves readability a lot.

@tyru Oh, that's right ><
Thanks! lol

Shouldn't this be closed now that there is #4634 ?

Sure👍


feat: support etcd storage && build: upgrade version to 0.1.4

Codecov Report

Merging #35 (9d22d72) into main (93948f9) will decrease coverage by 1.50%.
The diff coverage is 62.13%.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
@@            Coverage Diff             @@
##             main      #35      +/-   ##
==========================================
- Coverage   77.57%   76.07%   -1.50%
==========================================
  Files           9       10       +1
  Lines         758      861     +103
==========================================
+ Hits          588      655      +67
- Misses        114      138      +24
- Partials       56       68      +12

Files
Coverage Δ

stores/etcd.go
62.13% <62.13%> (ø)

... and 1 file with indirect coverage changes
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more


Invalid directory error

Hi! Hitting the following:
uncaught error Error: UnableToResolveError: Unable to resolve module react-native-notification-actions from /[path]/index.ios.js: Invalid directory /Users/node_modules/react-native-notification-actions

React seems to be looking for the module in Users/node_modules/react-native-notification-actions, when it is of course in [path]/node_modules/react-native-notification-actions
RN 0.18.0-rc
Any ideas?
Thanks! Looking forward to using (and hopefully contributing) to your module!

Related: https://github.com/facebook/react-native/issues/4968#thread-subscription-status
Unfortunately, the fixes there did not repair the error. Any insight into this @holmesal ? If not, might just close this issue and keep watching for another fix in that thread.

Oops - was referencing the module wrong! Seems to be only related to that issue insofar as the Users/ directory was the last place up the chain it looked for the module (see https://github.com/facebook/react-native/issues/4968#issuecomment-167662382).
Pushing a slight correction to the docs.

Fixed w/ merge of #2


Odd systematic IDs in all_coordinate_changes_file_comments.tsv

Hi Manu.
There are a few strange items in the systematic_id column of all_coordinate_changes_file_comments.tsv:
 "SPBC19G7.01c /controlled_curation=""term=disease associated, cancer; date=20060724"
 "SPBC19G7.01c /controlled_curation=""term=disease associated, cancer; date=20060724"
 *****
 *****
 Juan
 Juan

5826    kmr44   2019-09-29      II      "SPBC19G7.01c /controlled_curation=""term=disease associated, cancer; date=20060724"    msh2    CDS     removed complement(join(2344457..2347291,2347386..2347468,2347512..2347542))
5823    antonia 2019-09-27      II      "SPBC19G7.01c /controlled_curation=""term=disease associated, cancer; date=20060724"    msh2    CDS     added   complement(join(2344457..2347291,2347386..2347468,2347512..2347542))
4106    vw253   2017-06-28      II      *****           ncRNA   removed 1676220..1676335
4105    vw253   2017-06-27      II      *****           ncRNA   added   1676220..1676335
1909    antonia 2014-07-08      I       Juan            CDS     removed join(650488..650545,651193..651848)
1906    antonia 2014-07-07      I       Juan            CDS     added   join(650488..650545,651193..651848)

Some of these features existed in the contigs, but were removed as errors:
FT   CDS             join(650488..650545,651193..651848)
FT                   /systematic_id="Juan "

FT   ncRNA           1676220..1676335
FT                   /SO="SO:0000602"
FT                   /db_xref="EMBL:BK009173"
FT                   /product="U2 snRNA-specific guide RNA"
FT                   /systematic_id="*****"

I guess these two could be left, since they are clearly temporary ids. Alternatively we could filter on that column starting with SP.
The other one is a missing quote in the systematic_id that makes it span two rows:
FT                   /systematic_id="SPBC19G7.01c
FT                   /controlled_curation="term=disease associated, cancer;
FT                   date=20060724"

Since it was eventually corrected, I guess filtering on that column starting with SP would be enough for this case as well. I don't think it's worth changing the code that produces the files, since there are already many ifs / exceptions, and this is a problem in the input.


Fixed memory cycle caused by a strong delegate reference

Good catch

Not into master :(

Yes, my master...

Master of disaster 😂


[Feature Request] Ruffle support

It would be great if .swf files opened with Ruffle, like on danbooru.

Done in v1.9.0


Arnej/less wildcards for config producers 2

@gjoranv please review

@hmusum please take a look also


fixed .. concat bug in debian10's vim8.1

A bug happend in my NAS,  which is installed  debian10 with vim8.1.
This version of vim does not support string concat operator .., so I have replace thems with . , otherwise it would give me a . s:home errror message when do update.
Actually before the replacements,  neither vim8.1 in Ubuntu20.04  nor nvim-0.6+ in any system did not give me this error,  may be it is cause by a minor version update in vim8.1.

Thank you so much. I am happy hear that you use my plugin in your NAS.


Apply [] attribute to prefix hook imports with use

This PR applies the [<Hook>] attribute from Feliz to prefix hook imports with use when defined and when applied.
This is because of the way react-refresh detects custom hooks: functions that start with use and sometimes Fable will obscure it, hence the attribute.
Please check if your PR fulfills the following requirements:

[ ] Tests for the changes have been added (for bug fixes / features)
[ ] Docs have been reviewed and added / updated if needed (for bug fixes / features)
[ ] Build (fake build or .\build.cmd) on local branch was successful

Pull request type
Please check the type of change your PR introduces:

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, renaming)
[x] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] Documentation content changes
[ ] Other (please describe):

What is the current behavior?
After a hot reload, atom hooks no longer detect changes and I have to refresh the page to get them to work again.
What is the new behavior?
I still need to test this but I believe this will fix the react-refresh issue. If it doesn't I will try to create a full sample to repro the issue
Does this introduce a breaking change?

[ ] Yes
[x] No

Other information
Using latest version as of the time of the writing

@Shmew Hello, any reason this is not merged (maybe you didn't see it / didn't have the time yet) ?


Add DOCKER_CONFIG echo in the path that is being used

The previous addition of DOCKER_CONFIG check is not being called (yet), so add the same code to the path that is currently erroring.

Might also be useful to run a docker info somewhere before the push to see which docker username is being used.

Good idea, I'll add thay

The build timed out...is there an easy way to retrigger ci.j.io?


Fixed travis tests

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@f7465d3). Click here to learn what that means.
The diff coverage is 0%.

@@          Coverage Diff          @@
##             master   #5   +/-   ##
=====================================
  Coverage          ?   0%
=====================================
  Files             ?    2
  Lines             ?   85
  Branches          ?    0
=====================================
  Hits              ?    0
  Misses            ?   85
  Partials          ?    0

Impacted Files
Coverage Δ

main.go
0% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f7465d3...216b92f. Read the comment docs.


[Idea] - scrap tenant resolver middleware

Just an idea that I ended up implementing recently myself and seemed to work quite well. Consider this middleware: https://github.com/saaskit/saaskit/blob/master/src/SaasKit.Multitenancy/Internal/TenantResolutionMiddleware.cs
You can scrap this middleware entirely if:

Replace it with a "scoped" service, that does exactly what the middleware does, but in a Lazy<TTenant> such that the tenant is lazily resolved on first access of the Tenant property / method.

I think this then simplifies things a bit because:

TenantResolutionMiddleware can be dropped meaning less step for consumer.
People can inject the ITenantAccessor service whenever they need to access current Tenant (rather than inject TenantContext or TTenant.
You can change the implementation of ITenantAccessor so that it doesn't need to store the TenantContext in httpcontext - because its already a scoped service, it can just keep the TenantContext directly as a property. This means you could also remove the extensions methods for httpcontext that Get / Set the TenantContext from httpcontext - as it would no longer be stored there - it would be held on the ITenantAccessor, which unlike the middleware, is accessible anywhere the tenant needs to be accessed.

I do think the middleware concept is nice from a logging perspective to see which tenant (and other data) was resolved (for me from a database or distributed cache) prior to the request being dumped into MVC.  Every request in my app requires this tenant data so for us it makes sense to load and validate it up front.
That being said, I could see us writing the middleware that I described which just uses ITenantAccessor.
ITenantAccessor could also have a property bag outside of TenantContext that we could use for same request caching instead of using HttpContext.  We currently do this for some infrequently used but expensive data elements that should not live in TenantContext, but we do not want to compute twice on the same request.

We currently do this for some infrequently used but expensive data elements that should not live in TenantContext, but we do not want to compute twice on the same request

If you want to ensure some exoensive state is only loaded once, and only when its needed, per request, then im not sure why you dont register a scoped service in the container that provides that state as a lazy.

Yes, agreed.  Better separation of concerns at least.  My original thought was to combine the two since they were similar in their scope but a separate scoped service makes more sense overall.
Also, the reason for this data not going into TenantContext is because it's user / workstation / functionality configurations typically based on the specific workflow the user is currently in.  There are hundreds of workflow and config variations and it doesn't make sense to cache them as a unit in TenantContext.  Sometimes we need the config during processing and also in the view, in these cases I just don't want to hit the DB or Redis twice.

In the end, I spiked out my idea: Here is the code with a sample app: https://github.com/dazinator/Dotnettency.git and published nuget packages here: https://www.nuget.org/packages?q=DOTNETTENCY

@dazinator I'm in the process of upgrading SaasKit to csproj and then will look at some of the items we discussed. It would be good if we could incorporate your ideas into SaasKit - I'll have a look through your code tomorrow.

@benfoster - Ok no problem. You should be able to run the sample under kestrel, and hit it on localhost:5000 - 5003 to see how the various tenants behave. Per tenant middleware and Containers seem to be working ok. I will probably add in support for some more containers before I am done (like autofac and ninject).

@benfoster @dazinator would be nice if the SaaSKit supported the new asp.net core 2.0 kestrel pipeline as the performance improvement work the asp.net core 2.0 team have done is greate, also there are some breaking changes which may impact the SaasKit functionality

Do you have a link providing more details about this?
On Wed, 26 Jul 2017 at 01:30, Grahame Horner<EMAIL_ADDRESS>wrote:

@benfoster https://github.com/benfoster @dazinator
https://github.com/dazinator would be nice if the SaaSKit supported the
new asp.net core 2.0 kestrel pipeline as the performance improvement work
the asp.net core 2.0 team have done is greate, also there are some
breaking changes which may impact the SaasKit functionality
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/saaskit/saaskit/issues/77#issuecomment-317911781, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAyfqRN7osld0QSoLR-wdlEIAfoW7_FTks5sRoiOgaJpZM4OZUqU
.

@benfoster only the standard github/aspnet repo with announcements

@dazinator I've had a look your code - great job! It's certainly given me some ideas on how I can unify some of the SaasKit APIs.
However, I'm not yet sold on dropping the middleware since somewhere along the line you're going to need to await the resolver/accessor and I'd prefer to have the framework do this. Likewise, while I get the reasoning for making this lazy, in practice do you foresee many scenarios where you won't want to identify the tenant at the beginning of the request?

Maybe it depends. If staticfiles is added after multitenancy middleware you might be resolving tenant on some requests for no reason..
With the new razor pages stuff, you might have some simple content pages that are common to all tenants, so if that page is requested, you dont necessarily need the tenant to be resolved during that request.
To be honest ive just tried to be flexible here. You could still have the middleware sitting on top of the lazy, but it wouldnt be required in the pipeline in order to have a tenant resolved, it would just be optionally used in the pipeline if you know you always want to enforce the tenant resolution at a certain stage regardless of the type of request. I will probably end up calling it EnforceTenantResolution middleware or something.

One last thing

since somewhere along the line you're going to need to await the resolver/accessor

If TTenant is injected directly, the awaiting is handled by the framework at that point. I added that feature so you dont have to inject an ITenanrAccessor and await yourself.

@dazinator can you point me to the code where you're doing that?

Sure. https://github.com/dazinator/Dotnettency/commit/d99c39cb6837d268ebee4721aaa1ff039bc4d8e6

Unfortunately this is a blocking call. This is what I was trying to avoid.

Yep guilty as charged! :-)
Thinking through that - given that the task is lazy - guessing it would block om the first inject ( assuming it hadnt already been resolved earlier anywhere else in the request?)

I guess in terms of the blocking:

Potentially long block if tenant needs to be initialised (very first request on ap start)
Small block if: task not yet run (first inject of TTenant for request)
No block if: tenant is already resolved.

Think number 1 might be an issue. Number 2 isn't ideal but I can just about live with it given that it should be very small as the fully initialised tenant just needs to be grabbed from cache.
I'm going of this idea now.

I have decided in the end, to still support injection of TTenant, but just document that it has the potential to block. I'll advocate that ITenantAccessor should be used for non blocking access to the current tenant and leave it at that!

One other idea if you wanted to be really explicit about it would be to support the inject of Blocking<TTenant> which is just a wrapper around the TTenant instance resolved via the potentially blocking Task.Result... Meh.. probably overkill at this stage.

Or even.. allow Task to be injected!

Inject Task seems to work nicely: https://github.com/dazinator/Dotnettency/commit/a15eaa05566abe7efdef1e3468974da88869fee9#diff-dcaa3e511ec5996c9946ae81b027c8d1R16


add author and link to docs in nat spec

so this shows up in verified contracts

done in https://github.com/latticexyz/mud/pull/2333


Correction for raw affiliation Department of Mathematics, Western University

Correction needed for raw affiliation Department of Mathematics, Western University
raw_affiliation_name: Department of Mathematics, Western University
new_rors: 02grkyz14
previous_rors: 02agqkc58
works_examples: W4366083753;W4372286277
contact: ea9192a45edfcb5d5b467983d8175022:e6de89b398b7d3a9 @ uwo.ca

This issue was accepted and ingested by the OpenAlex team on 2024-12-21. The new affiliations should be visible within the next 7 days.


Make Guild#ban delete messages when specified

Please describe the changes this PR makes and why it should be merged:
The last if feels pretty hacky, there is most likely a much better solution.
It would also be possible to:

Remove days as accepted key and use 'delete-message-days' instead,
but the name is imo a rather poor choice.
A different conversion in RESTMethods, that would accept days.
Or something entirely different, that I didn't thought of.

Semantic versioning classification:

[ ] This PR changes the library's interface (methods or parameters added)

[ ] This PR includes breaking changes (methods removed or renamed, parameters moved or removed)

[ ] This PR only includes non-code changes, like changes to documentation, README, etc.

/tableflip


Toggle/switch support

🚀 Feature request
It would be nice to have support for a Switch or Toggle component
🧱 Problem Statement / Justification
ARIA switch role: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/switch_role
✅ Proposed solution or API
https://chakra-ui.com/docs/components/switch
https://mui.com/material-ui/react-switch/
↩ Alternatives
ARIA implementation https://react-spectrum.adobe.com/react-aria/useSwitch.html

I'm getting type errors when trying to use the new switch machine:
import * as zagSwitch from '@zag-js/switch';
import { useMachine, normalizeProps } from '@zag-js/react';

const [state, send] = useMachine(
    zagSwitch.machine({
      id,
      value: checked ? 'checked' : 'unchecked',
      onChange,
      label,
      disabled,
    }),
  );
  const api = zagSwitch.connect(state, send, normalizeProps);

Argument of type 'Machine<MachineContext, MachineState, AnyEventObject>' is not assignable to parameter of type 'MachineSrc<MachineContext, StateSchema, EventObject>'.
  Type 'Machine<MachineContext, MachineState, AnyEventObject>' is not assignable to type 'Machine<MachineContext, StateSchema, EventObject>'.
    The types of 'state.tags' are incompatible between these types.
      Type 'unknown[]' is not assignable to type '(string | undefined)[]'.
        Type 'unknown' is not assignable to type 'string | undefined'.ts(2345)

Argument of type 'State<MachineContext, StateSchema, EventObject>' is not assignable to parameter of type 'State'.
  Types of property 'value' are incompatible.
    Type 'string | null' is not assignable to type '"checked" | "unchecked" | null'.
      Type 'string' is not assignable to type '"checked" | "unchecked" | null'.ts(2345)


Expose the whole tls.Config setting

Identified flaky test failing. Will investigate after this PR. Merging now.


Add secondary telemetry to InlineController

Test plan
run extension in debug mode
check test eventlogs on dotcom for event emission
check bigquery for event reception

Could you elaborate the test plan a bit? "check bigquery" and "check test event logs on dotcom" is a bit of a leap for people who aren't doing it regularly. I guess that means you're signed in to dotcom? Spell it out so people can reproduce your test plan.
Needless to say, it would be great if this had an automated test.

It would be great to give a tiny bit of background about the motivation for this change, too, since it is not linked to an Issue. Yes, it adds another telemetry thing, but why are we doing that?

@dominiccooney The general background of this effort is supporting implementation of the new Telemetry framework for the #wg-event-logging-everywhere project. We want to enable event telemetry by default, our old event logging mechanism wouldn't be able to support this, the Telemetry Gateway solves that issue and allows us to scale.
Adding the secondary telemetry recording allows us to ensure that we're not losing any data before we deprecate the old system.
We do have automated tests that check loggedV2Events for expected captured events. Example from task-controller.test.ts:
await expect.poll(() => loggedEvents).toEqual(expectedOrderedEvents)
await expect
    .poll(() => loggedV2Events)
    .toEqual([
        'cody.auth/failed',
        'cody.auth/connected',
        'cody.command.edit/executed',
        'cody.recipe.fixup/executed',
        'cody.fixup.apply/succeeded',
    ])

I can add more robust tests for all of the different reconfigured feature names. I have generally been running these locally with a config flag to enable the export of events I'm testing. Any event that is being tested will flow into the event_logs table then be exported to a pub/sub which feeds the data into the Telligentsourcegraph project (telemetry.event_telemetry_raw is the landing location).

@dominiccooney some additional context:

Background: https://docs.sourcegraph.com/dev/background-information/telemetry
Introduction of new SDK: https://github.com/sourcegraph/cody/pull/1192
Issue for Nathan's work is https://github.com/sourcegraph/sourcegraph/issues/56284, but this is quite out of date - https://github.com/sourcegraph/cody/pull/1192 has more context than the issue 😅 I'll try and get around to updating that tomorrow

@nathan-downs , @bobheadxi thanks for those details. If the automated tests are enough it would be great for the test plan to show how to get that configuration and just run them.
CC @DanTup who is reordering some events to get a Windows integration test up, FYI. At some point we'll need the expected TelemetryV2 events in the right order.

I posted on #1517 but it would be neat if you had recordEvent return an object which then does the classic telemetry.log. It will give you more leverage that the logs actually line up, and make it easier to delete all the old logging later.

CC @DanTup who is reordering some events to get a Windows integration test up, FYI. At some point we'll need the expected TelemetryV2 events in the right order.

My changes are in https://github.com/sourcegraph/cody/pull/1535 which was mostly making the events more predictable (less unwanted requests during the test). After both of these land, I could update those tests to verify the TelemetryV2 events too (assuming they're available to verify in tests).

replaced by https://github.com/sourcegraph/cody/pull/2060


Adapter cable types supported on Linux-like OSes

According to the documentation, on Windows the cable needs to present a COM port and is addressed as such, whereas on Unix-like OSes (Linux and Mac) only FTDI adapters are supported (as far as I understand), which are addressed by the adapter ID.
Looking at the source code, I am getting a different picture: the string submitted on the command line is matched against a regex to determine if it is an alphanumeric string of exactly 8 characters. If so, it is a valid FTDI adapter ID and treated as such, i.e. kw1281test will open an FTDI interface with the given ID. Anything else is treated as a serial port.
Therefore, e.g. COM3 would cause kw1281test to open a serial port with that name, which is what happens on Windows.
However, /dev/ttyUSB0 would also be treated as a standard serial port. Would I be able to make use of that on Linux/MacOS if my adapter presents itself as a serial port, by specifying its filesystem path, and thus be able to use any adapter that the OS sees as a serial port?
If so, then CH340-based adapters should work on Linux – I have successfully used a USB to RS232 adapter based on the CH340 for serial console access on Linux (Ubuntu 18.04, 20.04 an 22.04 – the latter required a modification because of a Braille terminal using the same USB ID as my adapter, preventing it from being recognized as a serial port). In that case, the documentation should be amended to include that any adapter that creates a /dev/tty* node should work on Linux.
PS: I have taken the risk and just ordered a USB KKL cable – no idea what chipset it uses, but I will report back on my Linux experience once it arrives.

The reason that only FTDI adapters are supported under Linux is that the kw1281 protocol wakes up a module in the car by sending the address of the module at 5 baud and then waiting to receive an acknowledgement at 9600 or 10400 baud. 5 baud is not supported by most serial APIs and even when supported, switching from 5 baud to a higher baud rate often takes too much time reprogram the UART chip timing, which results in missing the acknowledgement. To work around that problem, kw1281test simulates the 5 baud wakeup by sending a break or clearing a break on the serial line for 200ms per bit. This works fine on Windows but the Linux/macOS serial port API does not support sending a break signal for a specific amount of time:
https://linux.die.net/man/4/tty_ioctl
"Sending a break
TCSBRK
int arg
Equivalent to tcsendbreak(fd, arg).
If the terminal is using asynchronous serial data transmission, and arg is zero, then send a break (a stream of zero bits) for between 0.25 and 0.5 seconds. If the terminal is not using asynchronous serial data transmission, then either a break is sent, or the function returns without doing anything. When arg is nonzero, nobody knows what will happen.
(SVr4, UnixWare, Solaris, Linux treat tcsendbreak(fd,arg) with nonzero arg like tcdrain(fd). SunOS treats arg as a multiplier, and sends a stream of bits arg times as long as done for zero arg. DG/UX and AIX treat arg (when nonzero) as a time interval measured in milliseconds. HP-UX ignores arg.)"
At least that's my experience when I tried to get kw1281test working under macOS and Linux using a regular serial port. The FTDI Linux/macOS driver supports more precise timing of the break signal. Of course if your experience is different and you successfully get it to work with a CH340 cable, please let me know.

Thanks for the clarification. Great there are people like you who have both car and IT knowledge – FOSS car diagnostics tends to be a niche because most FOSS devs don’t seem to have cars, and most DIY car mechanics seem to shun computers.
Anyways, I have little experience with that kind of low-level bit banging. Having (re-)read the part on KW1281 initialization at https://www.blafusel.de/obd/obd2_kw1281.html, a few approaches came to my mind:

Is the set/clear break method how other tools (VAS1551, VCDS) solve the baud rate switch as well? Although VCDS only supports Windows, it uses the same cables and would face the same challenges. Do these tools use timed breaks, or do they approach this differently?
Have you tried asking around sites like Stack Overflow or Unix Stack Exchange if others have encountered and solved this problem?
Would it be possible to simulate a fraction of the current baud rate not by sending and clearing a break but by repeating the same bit multiple times? Start/stop bits would get in the way (the actual communication is 8n1, hence no parity bit) – how do the remote ends deal with “impurities” several orders of magnitude shorter than the duration of a bit? Are they ignored or would they be taken as an indication of an ongoing data transmission at a higher baud rate?
Assuming the serial driver supports the low baud rate, we start at 5bps and then switch: As I understand the description by Hex Microsystems (linked from blafusel.de), the PC acknowledges each byte of the ECU’s response with its complement (save for the three bytes preceding the first block, of which only the last byte is acknowledged), and responds to each block with an ACK block. What would happen if the PC fails to acknowledge the start of the transmission?

Will the ECU retransmit it? (Which is why I imagine this whole acknowledgement business is there in the first place.) In that case the adapter could take a little longer for the baud rate switch and wait for the retransmission.
Or will the ECU simply stop talking until the PC sends something – say, a NACK block? Then we could listen for the ECU response, and if none arrives in a certain timeframe (or we get garbage that doesn’t start with a sync byte), ask the ECU to repeat the transmission.
Or will the ECU close the connection and stop communicating with the PC altogether until it is woken up again? In that case this approach won’t work.

@mvglasow
Sorry for interrupting. I am Japanese and not fluent in English so I am using DeepL translation.
I apologize if I didn't make myself understood or if I sounded rude.
It would be very much appreciated if you could get it working on CH340, so I don't mean to put a damper on that.
I know this is advice in a different direction, but if you want to be sure of getting a KKL cable using FT232RL, there is a way to combine an RS232 KKL cable with FT232RL.
RS232 KKL Interface
https://www.ecufix.com/shop/index.php?main_page=product_info&products_id=181
https://aliexpress.com/item/1005001464817648.html
USB to RS232 with FT232
https://www.amazon.com/DSD-Tech-SH-RS232A-adaptador-5-9/dp/B07MYYCMJT
https://www.amazon.com/DSD-TECH-SH-U20A-Adapter-FT232RL/dp/B083TFL4KS/
I am operating with the above ECUFix cable and SH-RS232A combination and have confirmed in the past that it also works with FT232BM and a cable using an external EEPROM.
We have run it with Windows 10 and Raspberry Pi (built by myself for 64bit Bullseye), and it works fine because we were able to get SKC of Polo 9N3 with Windows, and get SKC and read/rewrite cluster EEPROM with Golf mk4 on Raspberry Pi.
I think this is the most reliable method, although the cables are longer and the appearance is a bit ugly.
So, I hope that the operation with CH340 will be successful.
Sincerely yours

@accept Thanks, getting a serial KKL cable and separate USB-serial adapter really sounds like an option. Fortunately, I got my cable these days and it is FTDI.
Nonetheless, having to disable USB-serial kernel modules or messing with USB device identification is not particularly nice. So, if the regular serial driver stack were supported on Linux, that would be a huge plus.

Back on topic: I have had a chance to look at another tool, buried in the source code of AndrOBD and to my knowledge never published as a tool in its own right – however the author claims to have used it successfully.
It is written in Java and relies on the RXTX library for serial communication, which ultimately translates into calls to the serial API.
Said tool sends the bits of the slow-init sequence by turning break on or off, then sleeping for 200 ms before sending the next bit. As I understand the manpages on Ubuntu 22.04, this should translate into calls to tty_ioctl with TIOCSBRK and TIOCCBRK. (Still working on getting it to build, then I can report on success.)
Alternatively, tty_ioctl also has TCSBRKP:

So-called  "POSIX  version"  of  TCSBRK.   It treats nonzero arg as a time interval measured in deciseconds, and does nothing when the driver does not support breaks.

Calling that with an arg value of 2 should have the desired effect.

Thanks for doing the research on this. I'm pretty sure I tried this a year or two ago and discovered that macOS would not allow precise timing of TIOCSBRK/TIOCCBRK (I have a logic analyzer so I can see the timing of what shows up on the k-line when I run kw1281test). When I get some free time I'll try again and see if I get better results.

Back on topic: I have had a chance to look at another tool, buried in the source code of AndrOBD and to my knowledge never published as a tool in its own right – however the author claims to have used it successfully.

I think AndrOBD may not be that useful for examples of how to implement the 5-baud wakeup. AndrOBD relies on an ELM327 adapter, which has built-in 5-baud initialization. If you discover otherwise, please let me know and point me at the relevant source code.

Alternatively, tty_ioctl also has TCSBRKP:

So-called  "POSIX  version"  of  TCSBRK.   It treats nonzero arg as a time interval measured in deciseconds, and does nothing when the driver does not support breaks.

Calling that with an arg value of 2 should have the desired effect.

That does seem to be implemented in the Linux source code, so I'll see if I can implement it:
https://github.com/torvalds/linux/blob/master/drivers/tty/tty_io.c
	case TCSBRKP:	/* support for POSIX tcsendbreak() */
		return send_break(tty, arg ? arg*100 : 250);

I think AndrOBD may not be that useful for examples of how to implement the 5-baud wakeup. AndrOBD relies on an ELM327 adapter, which has built-in 5-baud initialization. If you discover otherwise, please let me know and point me at the relevant source code.

AndrOBD proper relies on the ELM327 and therefore does not need any of the baud-switching tricks. However, it is built on top of a JRE application the author built a few years earlier, which is intended for use with a dumb USB or serial KKL cable and has support for both OBD-II and KW1281. Some parts of the source code were reused in AndrOBD; the app itself can be found in the library dir. The code is no longer 100% complete and requires a few fixes to build properly, but the author claims to have used it successfully to diagnose his old car with it. The code I am referring to is not used in AndrOBD. It is located here: https://github.com/fr3ts0n/AndrOBD/blob/master/library/src/main/java/com/fr3ts0n/prot/gui/KLHandler.java#L233

New LinuxInterface may solve the problem.
See PR #73 . Thx


Issues adding resource pool through swagger

When trying to add a resource pool using the swagger API, default_storage and default are missing from the request body.
A quota is provided in the request body. When posted, I get this error:
{
  "error": {
    "code": 1422,
    "message": "A default resource pool cannot have a quota."
  }
}

When I remove the quota I get this error:
{
  "error": {
    "code": 1422,
    "message": "The quota in the resource pool is malformed."
  }
}

I don't understand what actually needs to be in the request body.

Testing this on 0.12.0 on dev.renku.ch.
The example payload now includes default_storage and default.
The payload now is as follows by default
{
  "quota": {
    "cpu": 50,
    "memory": 250,
    "gpu": 10
  },
  "public": true,
  "default": false,
  "classes": [
    {
      "name": "resource class 1",
      "cpu": 1.5,
      "memory": 2,
      "gpu": 0,
      "max_storage": 10,
      "default_storage": 2,
      "default": true
    },
    {
      "name": "resource class 2",
      "cpu": 4.5,
      "memory": 10,
      "gpu": 2,
      "max_storage": 10,
      "default_storage": 2,
      "default": false
    }
  ],
  "name": "resource pool name"
}

The default includes a quota and can be created without a problem.


adapter-netlify: Support multiValueHeaders key in returned object from handler function

Is your feature request related to a problem? Please describe.
Netlify Functions allow a multiValueHeaders key in the returned object of the handler function. This key is specifically made for array values.
For example, as the Svelte-kit docs say, in order to set multiple cookies in a response, you can do the following in an endpoint:
return {
  headers: {
    'set-cookie': [cookie1, cookie2]
  }
};

However, this fails when deployed to Netlify. In order to set multiple cookies in a Netlify function, you have to do this:
return {
  multiValueHeaders: {
    'set-cookie': [cookie1, cookie2]
  }
}

This is documented here: https://docs.netlify.com/functions/build-with-javascript/#synchronous-function-format.
Describe the solution you'd like
A way to set multiple cookies with the Netlify adapter, ideally keeping the same API as described above. The adapter should check for array values in the rendered headers object and transfer them in a multiValueHeaders key in the returned response.
Describe alternatives you've considered
There isn't really any alternatives to this, as the only way to set multiple cookies in a response with Netlify functions is through this key.
How important is this feature to you?
Very! Setting multiple cookies is supposed to be a supported feature in Svelte-kit. Also, most auth systems are going to be able to do this.

Is there anything else to discuss before I start to code the fix?

@JeanJPNM Oh my god thank you so much for this! I wanted to get to it but couldn't find the time. I can't understate how good that timing is.


Add ./mach build-stable to linux-dev builder.

This is the second and last PR related to this issue in servo: https://github.com/servo/servo/issues/11806
This PR added the ./mach build-stable sub-command: https://github.com/servo/servo/pull/11945

This change is

I discussed this with Jack and we concluded that the servo/servo repository is unlikely to get code that we want to build on stable Rust other than ports/geckolib and its dependencies. And with https://github.com/servo/servo/pull/12263 landed, we can do so now.
So I’m going to remove the ./mach build-stable sub-command (making this PR unnecessary) and instead make ./mach build-geckolib use the stable compiler using the bootstrapping and setup that you introduced in https://github.com/servo/servo/pull/11945. Thanks again for that!


Feat: Allow setting depends_on_past explicitly instead of inferring it

ref: https://tobiko-data.slack.com/archives/C044BRE5W4S/p1710202877518519?thread_ts=1710201667.408719&cid=C044BRE5W4S
Sometimes, you need an incremental model to run sequentially.
Setting batch_size: 1 makes a task per interval, but it still runs batches in parallel based on the concurrent_tasks setting of the gateway connection.
There is an existing depends_on_past property that triggers sequential execution but it's currently derived and cant be set by the end user.
This PR allows it to be set as part of the model Kind

As I mentioned previously, the depends_on_past has implications beyond just sequential execution of batches. It also impacts whether the plan start is appropriate for this model and if not a model can be excluded from the plan when iterating in dev.
That's why I'm not sure I like the idea of somebody setting this property at will. It makes the configuration more complex, it's easy to misuse and it feels rather redundant given the mechanisms we have in place already that can achieve the same behavior.
How do you like the idea of including the model name as part of the depends_on attribute manually to signal that you want to opt into the depends_on_past behavior? I've made this behavior possible here: https://github.com/TobikoData/sqlmesh/pull/2323
Let me know what you think.

Sure, that would work too, I didn't think of that.
I guess both methods are an indirect way of achieving the goal of making a model run sequentially.
To make the intent more clear, would a sequential: True property (that under the hood sets depends_on -> self) be appropriate?
Otherwise i'm happy to go with your suggestion since you have the wider context :)

@erindru yes, I like sequential! I think it captures the intention much better


Way to query if any scope is set

Consider changing the API of this:
https://github.com/smartpde/neoscopes/blob/2654f275bf3fb3d3a255b7925464d4cade5b9083/lua/neoscopes.lua#L241
To return nil instead of error when there is no scope set. Otherwise, there's no way to tell safely whether set_current has been called at least once.

Sure, that makes sense, thanks!


util/cmdline: rename "OEM ID" to "Platform ID"

This switches from "coreos.oem.id" to "coreos.platform.id" in
non-legacy mode. The OEM key lookup is temporarily kept to ease RHCOS
transition.
Ref: https://github.com/coreos/coreos-assembler/issues/428

/cc @bgilbert @dustymabe @cgwalters

This switches from "coreos.oem.id" to "coreos.platform.id"

shouldn't it be ignition.platform.id ? ref: https://github.com/coreos/ignition-dracut/pull/45

@dustymabe indeed, I didn't see that PR. Renamed.

Per https://github.com/coreos/coreos-assembler/pull/433#issuecomment-476177645, maybe we should go ahead and accept only the new flag in non-legacy mode.

Per coreos/coreos-assembler#433 (comment), maybe we should go ahead and accept only the new flag in non-legacy mode.

That makes sense IOW the media creator will generate both coreos.oem.id and ignition.platform.id so the consumer software doesn't have to handle both cases.

I'm going to hold this a bit until we sort out the approach in https://github.com/coreos/coreos-assembler/pull/433. I'm fine either ways, I thought that an overlap window here would make easier the lockstep transition on RHCOS side.

https://github.com/coreos/coreos-assembler/pull/433 landed adding both flags. As requested by @bgilbert, I reworked this PR to drop the overlapping/legacy fallback. PTAL, thanks!


Converter needs type cast in strong mode

I have this code:

I don't understand why I need the typecast, as I think that Converter implements StreamTransformer.
This silences the error:

SYSTEM_ENCODING is typed as SystemEncoding, which extends Encoding, which extends Codec<String, List<int>> which has a getter Converter<List<int>, String> get decoder.
The only difference I can see is that you use dynamic as the output type of the decoder instead of String.

I don't know if it's the last lack of type parameters that confuses the analyzer in strong mode - it shouldn't be a problem in normal Dart.

Yup, that would be it. StreamTransfomer<dynamic, dynamic> to StreamTransfomer<List<int>, dynamic> is not a safe cast.

Duplicate of #25698.


Added 16:9 aspect ratio option to actions

This commit adds a Set aspect ratio button to actions which sets the height of the window so that the aspect ratio will be 16:9. This is useful for consuming media (like Netflix) which is still usually has that ratio.

Very nice addition!


UI for managing credentials

Proposed Feature
Kargo needs credentials for things like:

git repositories
container repositories

Since they are Secrets, it becomes tricky to manage via GitOps. The UI should provide an imperative way to:

input credentials into a project namespace
view what credentials are available (not the actual sensitive content) and for which repositories

Motivation
Although managing credentials declaratively with things like external-secrets would be preferred, having an imperative way to do this would help with people getting started with kargo, developer self-service, etc...
Plus, having visibility in the UI to what credentials are available would be useful in and of itself.
Suggested Implementation
In the project view, have a credentials tab where this could be inputted and listed, but without exposing/returning any sensitive content.

Additional requirements:
If global credentials are set. The UI should also present this as well.

this could be a blocker for customer demo

Reopening since UI work not done

Sorry... it was quite by accident that I listed #1566 as closing this issue.

This was closed by #1710


Fix: Selected Xero bank account is not highlighted with a green checkmark

Explanation of Change

Fixed Issues

$ https://github.com/Expensify/App/issues/52699
PROPOSAL: https://github.com/Expensify/App/issues/52699#issuecomment-2483631521
Tests

Open App > Go to workspace settings > Accounting.
Connect to Xero.
After connecting to Xero, click Export.
Click Xero bank account.

Verify that: The selected Xero bank account (displayed on the previous page) should be highlighted with a green checkmark.

[x] Verify that no errors appear in the JS console

Offline tests

N/A
QA Steps

Open App > Go to workspace settings > Accounting.
Connect to Xero.
After connecting to Xero, click Export.
Click Xero bank account.

Verify that: The selected Xero bank account (displayed on the previous page) should be highlighted with a green checkmark.

[x] Verify that no errors appear in the JS console

PR Author Checklist

[x] I linked the correct issue in the ### Fixed Issues section above
[x] I wrote clear testing steps that cover the changes made in this PR

[x] I added steps for local testing in the Tests section
[x] I added steps for the expected offline behavior in the Offline steps section
[x] I added steps for Staging and/or Production testing in the QA steps section
[x] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
[x] I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).

[x] I included screenshots or videos for tests on all platforms
[x] I ran the tests on all platforms & verified they passed on:

[x] Android: Native
[x] Android: mWeb Chrome
[x] iOS: Native
[x] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[x] MacOS: Desktop

[x] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
[x] I followed proper code patterns (see Reviewing the code)

[x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
[x] I verified that comments were added to code that is not self explanatory
[x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[x] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method

[x] If any non-english text was added/modified, I used JaimeGPT to get English > Spanish translation. I then posted it in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:

[x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
[x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[x] I verified the JSDocs style guidelines (in STYLE.md) were followed

[x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[x] I followed the guidelines as stated in the Review Guidelines
[x] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
[x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[x] I verified that if a function's arguments changed that all usages have also been updated correctly
[x] If any new file was added I verified that:

[x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory

[x] If a new CSS style is added I verified that:

[x] A similar style doesn't already exist
[x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))

[x] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[x] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
[x] If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:

[x] I verified that all the inputs inside a form are aligned with each other.
[x] I added Design label and/or tagged @Expensify/design so the design team can review the changes.

[x] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[x] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

Reviewer Checklist

[x] I have verified the author checklist is complete (all boxes are checked off).
[x] I verified the correct issue is linked in the ### Fixed Issues section above
[x] I verified testing steps are clear and they cover the changes made in this PR

[x] I verified the steps for local testing are in the Tests section
[x] I verified the steps for Staging and/or Production testing are in the QA steps section
[x] I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)

[x] I checked that screenshots or videos are included for tests on all platforms
[x] I included screenshots or videos for tests on all platforms
[x] I verified tests pass on all platforms & I tested again on:

[x] Android: Native
[x] Android: mWeb Chrome
[x] iOS: Native
[x] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[x] MacOS: Desktop

[x] If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
[x] I verified proper code patterns were followed (see Reviewing the code)

[x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
[x] I verified that comments were added to code that is not self explanatory
[x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[x] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
[x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
[x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[x] I verified the JSDocs style guidelines (in STYLE.md) were followed

[x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[x] I verified that this PR follows the guidelines as stated in the Review Guidelines
[x] I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
[x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[x] If a new component is created I verified that:

[x] A similar component doesn't exist in the codebase
[x] All props are defined accurately and each prop has a /** comment above it */
[x] The file is named correctly
[x] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[x] The only data being stored in the state is data necessary for rendering and nothing else
[x] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[x] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[x] All JSX used for rendering exists in the render method
[x] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[x] If any new file was added I verified that:

[x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory

[x] If a new CSS style is added I verified that:

[x] A similar style doesn't already exist
[x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)

[x] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[x] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
[x] If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:

[x] I verified that all the inputs inside a form are aligned with each other.
[x] I added Design label and/or tagged @Expensify/design so the design team can review the changes.

[x] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[x] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
[x] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: Native
https://github.com/user-attachments/assets/2eaaf6a8-a115-4b9e-9ac2-2d1710e415b2

Android: mWeb Chrome
https://github.com/user-attachments/assets/50fc913f-94b2-4c41-9713-5fea536e4900

iOS: Native
https://github.com/user-attachments/assets/6cba1148-5cf9-4353-b9f8-8f1df7e72578

iOS: mWeb Safari
https://github.com/user-attachments/assets/aba2ad01-8ecc-4e55-ab4b-581dde2a72dd

MacOS: Chrome / Safari
https://github.com/user-attachments/assets/9f863be2-ee21-4c7c-8372-37f175e2f483

MacOS: Desktop
failed to login with error 529

Click Xero bank account.
Verify that: The selected Xero bank account (displayed on the previous page) should be highlighted with a green checkmark.

@twilight2294 I think we need to limit this step to the default value case when the user didn't select a value before.

Great!

:hand: This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.


Checkin some test vectors so we can prove interop

With inputs and outputs, we can better establish cross language interop.
Here is an example:
{
    "@context": [
      "https://www.w3.org/2018/credentials/v1",
      "https://www.w3.org/2018/credentials/examples/v1"
    ],
    "id": "http://example.gov/credentials/3732",
    "type": [
      "VerifiableCredential",
      "UniversityDegreeCredential"
    ],
    "issuer": "did:web:vc.transmute.world",
    "issuanceDate": "2020-03-10T04:24:12.164Z",
    "credentialSubject": {
      "id": "did:example:ebfeb1f712ebc6f1c276e12ec21",
      "degree": {
        "type": "BachelorDegree",
        "name": "Bachelor of Science and Arts"
      }
    }
  }

 {
      "@context": [
        "https://www.w3.org/2018/credentials/v1",
        "https://www.w3.org/2018/credentials/examples/v1"
      ],
      "id": "http://example.gov/credentials/3732",
      "type": [
        "VerifiableCredential",
        "UniversityDegreeCredential"
      ],
      "issuer": "did:v1:test:nym:z6MkhdmzFu659ZJ4XKj31vtEDmjvsi5yDZG5L7Caz63oP39k",
      "issuanceDate": "2020-03-10T04:24:12.164Z",
      "credentialSubject": {
        "id": "did:key:z6MkjRagNiMu91DduvCvgEsqLZDVzrJzFrwahc4tXLt9DoHd",
        "degree": {
          "type": "BachelorDegree",
          "name": "Bachelor of Science and Arts"
        }
      },
      "proof": {
        "type": "Ed25519Signature2018",
        "created": "2019-12-11T03:50:55Z",
        "verificationMethod": "did:v1:test:nym:z6MkhdmzFu659ZJ4XKj31vtEDmjvsi5yDZG5L7Caz63oP39k#z6MkiukuAuQAE8ozxvmahnQGzApvtW7KT5XXKfojjwbdEomY",
        "proofPurpose": "assertionMethod",
        "jws": "eyJhbGciOiJFZERTQSIsImI2NCI6ZmFsc2UsImNyaXQiOlsiYjY0Il19..uzx8px7eGewfc5Qa-GN1qjcYZH4nX3PBBaFW22x_573nnMu_Uzn6oT6ITL54QEk08XCqCE2OX2L-8ovsffXPDg"
      }
    }

Two test vectors added so far https://github.com/decentralized-identity/JcsEd25519Signature2020/tree/master/signature-suite-impls/test-vectors


Create "Team Norms" Doc

Create a "team norms" doc that details how the team will collaboratively develop the Grace Hopper application.

#3


Weird rounding function

DPoR reward is affected by this very weird function. Discovered by am2on on IRC. It makes reward amount always end with decimals "0124".

So I noticed something interesting -- every single time I get a DPOR reward, it always ends in "0124". > Look: https://i.imgur.com/JjsrtCp.png

Relevant code main.cpp:2053
std::string sTotalSubsidy = RoundToString(CoinToDouble(nTotalSubsidy)+.00000123,8);
if (sTotalSubsidy.length() > 7)
{
      sTotalSubsidy = sTotalSubsidy.substr(0,sTotalSubsidy.length()-4) + "0124";
      nTotalSubsidy = cdbl(sTotalSubsidy,8)*COIN;
}

Good find!
the length of TotalSubsidy is always more then 7 in length. Am I reading that correct? Rounds to 8 decimal places and adds 0.00000123 to maintain 8 decimal places in case it rounds to less then 8 decimal places? if the length is greater the 7 (which is always) it removes last 4 of the length and add "1024"
X.XXXXXXXX -> X.XXXX0124
XX.XXXXXXXX -> XX.XXXX0124
XXX.XXXXXXXX -> XXX.XXXX0124
XXXX.XXXXXXXX -> XXXX.XXXX0124
I don't see why you would round it up just to have the if statement be true to remove last 4 to add "0124" I understand that if the ending decimal place no matter where in the string is a 0 it is not kept and could understand a +0.00000001 if the length is less then 10 making sure you always have least 0.00000001 to maintain 8 decimal places.
seems like a patch job gone somewhere beyond?

It seems to be used to determine if a transaction is interest or PoR, rpcwallet.cpp:
bool IsPoR2(double amt)
{
    std::string sAmt = RoundToString(amt,8);
    if (sAmt.length() > 8)
    {
        std::string suffix = sAmt.substr(sAmt.length()-4,4);
        if (suffix == "0124" || suffix=="0123")
        {
            return true;
        }
    }
    return false;
}

... snip
std::string type = IsPoR2(CoinToDouble(r.amount)) ? "POR" : "Interest";
{
    entry.push_back(Pair("Type", type));
}

Surely there must be a more straight forward way of doing that?

interesting way to do it but makes sense. Might be another way better but My guess is because there is no proof-of-research flags in the transactions this is a easy way for them to be identified i guess. the transactions it checks least from me coping the transaction ids in transaction history just shows inputs/outputs but nothing i can tell what is what really. And at end of it shows the mint as interest with type = interest.

More research is needed. When I doubleclick por transaction, it shows me the id and stuff. But in getblock x true output, this por transaction is actually the second one and without hashboinc. The coinbase one with hashboinc is first and it has zero input and output.

@tomasbrod yea i noticed that as well. I was gonna original post saying use the hashbonic till i realized the second one didn't have that information. so that one went right out the door.

Would it be to much to pull the coinbase transaction as well with the one we are checking and see if the grc address in hashbonic ismine and then if so if Research subsidy field is 0.00? if it is not 0.00 it would be PoR and if it is 0.00 its interest block. the hashbonic has a fields for a grc address attached to wallet (this is never the same one i staked on). But the research subsidy field is always there even if your an investor.

Would it be to much to pull the block information verify if GRCaddress ismine and read the flags of the type of block? whether proof-of-research is included in the flags or not?

Something consistent in both researcher and investor is GRCaddress. and the research subsidy field in hashboinc.
Just some ideas. It would of defiantly been better if there was a flag in the transaction from the beginning of time which stated whether it was PoR or Interest.

Keeping this open as a reminder for improvements to the transaction dialog.

PR to deal with this is merged. Closing.


Containerize the executor service

Write a Dockerfile to build the image for this service. Automatically test the image and push to ECR.

Closing this issue because the executor does not need to be containerized


Fix problem with $$ not being put around the correct items in a LaTeX equation

For #8673
Added a bunch of unit tests (in particular the one that was failing).

Codecov Report

Merging #8707 into master will decrease coverage by 0.33%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #8707      +/-   ##
==========================================
- Coverage   59.17%   58.83%   -0.34%
==========================================
  Files         521      521
  Lines       23985    24010      +25
  Branches     3874     3874
==========================================
- Hits        14192    14127      -65
- Misses       8876     8959      +83
- Partials      917      924       +7

Impacted Files
Coverage Δ

...science-ui/interactive-common/latexManipulation.ts
100% <100%> (+6.25%)
:arrow_up:

src/client/testing/serviceRegistry.ts
48.64% <0%> (-46.85%)
:arrow_down:

src/client/testing/codeLenses/main.ts
44.44% <0%> (-33.34%)
:arrow_down:

...t/datascience/jupyter/jupyterDataRateLimitError.ts
66.66% <0%> (-33.34%)
:arrow_down:

src/client/formatters/serviceRegistry.ts
75% <0%> (-25%)
:arrow_down:

...ience/jupyter/jupyterDebuggerRemoteNotSupported.ts
75% <0%> (-25%)
:arrow_down:

src/client/common/utils/platform.ts
64.7% <0%> (-23.53%)
:arrow_down:

...cience/jupyter/jupyterDebuggerNotInstalledError.ts
60% <0%> (-20%)
:arrow_down:

src/client/common/utils/icons.ts
83.33% <0%> (-16.67%)
:arrow_down:

src/client/api.ts
78.57% <0%> (-14.29%)
:arrow_down:

... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1782b75...cbe8af3. Read the comment docs.


ip addresses for deleted services are not removed from RancherDNS in kubernetes cluster

Rancher Version:
1.0.0
Docker Version:
1.10.3
OS and where are the hosts located? (cloud, bare metal, etc):
rancheros, virtualbox
Setup Details: (single node rancher vs. HA rancher, internal DB vs. external DB)
single node rancher
Environment Type: (Cattle/Kubernetes/Swarm/Mesos)
Kuberentes
Steps to Reproduce:

setup a Kubernetes environment, wait for kubernetes cluster get ready

deploy the guestbook example, verify it works. check the rancher dns answer for redis-master service.

delete redis-master service through kubectl

recreate the redis-master service, and recheck the rancher dns answer for redis-master service.

Results:
2 IP addresses were presented in the answer.json for redis-master service: the one for the deleted service and one for the current service.
The IP for the deleted service should be removed. Otherwise, the service discovery through DNS will be broken. half of the time the client will get the IP of the deleted service.
Expected:
only one IP address was presented in the answer.json for redis-master service: the one for the current redis-master service.
We can't use Rancher created kubernetes cluster due to this issue. We have to go through the painful process to create the kubernetes cluster from scratch.

@bnusunny I can't reproduce the issue with the steps given on the latest build; could you check the steps below to see if anything is missing. Also can you check if when you remove redis-master service, it gets removed from rancher db:
select id, name, state from service where name='redis-master';

Created guestbook app from catalog.
Checked dns resolution for redis-master service:

root@bc2616652247:/# dig redis-master.default.svc.cluster.local.

.............

	IN A

;; AUTHORITY SECTION:
.			1654	IN	SOA	a.root-servers.net. nstld.verisign-grs.com.<PHONE_NUMBER> 1800 900<PHONE_NUMBER>0

;; Query time: 0 msec
;; SERVER: <IP_ADDRESS>#53(<IP_ADDRESS>)
;; WHEN: Mon May 23 17:46:03 UTC 2016
;; MSG SIZE  rcvd: 131

Added redis-master service back (via UI, using the service template below):

apiVersion: v1
kind: Service
metadata:
  creationTimestamp: 2016-05-23T17:46:27Z
  labels:
    name: redis-master
  name: redis-master
  namespace: default
  resourceVersion: "469"
  selfLink: /api/v1/namespaces/default/services/redis-master
  uid: 4693db8a-210e-11e6-a8a2-02f81793ba03
spec:
  ports:
  - port: 6379
    protocol: TCP
    targetPort: 6379
  selector:
    name: redis-master
  sessionAffinity: None
  type: ClusterIP
status:
  loadBalancer: {}

verified that redis-master service is resolvable to the new ip address now:
dig redis-master.default.svc.cluster.local.
...........
;; ANSWER SECTION:
redis-master.default.svc.cluster.local.	1 IN A	<IP_ADDRESS>

..........

checked answers.json file as well, it has just one ip address for redis-master service:
,
		"redis-master.default.svc.cluster.local.": {
			"answer": ["<IP_ADDRESS>"]
		},

Here is the snippet from rancher db showing that the old rancher-master service was removed, and new one recreated with a different vip. And that vip is the vip I saw in final dns resolution:
mysql> select id, name, state, vip from service where name='redis-master' and id > 20;
+----+--------------+---------+---------------+
| id | name         | state   | vip           |
+----+--------------+---------+---------------+
| 22 | redis-master | removed | <IP_ADDRESS> |
| 23 | redis-master | active  | <IP_ADDRESS>  |
+----+--------------+---------+---------------+

@sangeethah could you test it on dev4?

Followed the above steps and verified on master.


constructor to perform mass-assignment

I want to integrate virtus into our existing application as a replacement for ActiveModel form backends. Since we use Rails it would be convenient to have a mass-assignment functionality backed into virtus. Is there anything planned in that direction?

I haven't decided yet to be honest. I'm not sure if Virtus should be responsible for filtering out user input. I treat this library only as something which allows you to define your models via attributes API + it comes with coercion system.
"Mass-assignment security" is something that you only need in special cases where you have to deal with user input which can contain, well, basically anything. It seems to me like filtering out params is something that should be done by one of the web-framework 'layers'.
Even if we agree to build this into Virtus I'd prefer to have it a separate module that you can include only if you need this.
@dkubb Again, WDYT?
@emmanuel Don't you think conformitas would be a great place to deal with mass-assignment security?

I wasn't refering to mass-assignment security I just meant to be able to do something like:
class Example
  include Virtus

  attribute :name, String
  attribute :age, Integer
end

Example.new :name => 'test', :age => 10

without protection or anything like it. Of course I could extract the common constructor into a module and just have that code in my application but I wasn't sure if it should be a part where virtus could assist.

@senny oh! sorry - this is already built into Virtus: https://github.com/solnic/virtus/blob/master/spec/unit/virtus/class_methods/new_spec.rb#L13

@solnic sorry for the confusion and thanks for the hint :)

@senny no problem; at least this reminded me about mass-assignment security :)

regarding mass-assignment I like the approach described here: https://github.com/emmanuel/conformitas

@senny yup, me too. It seems like a proper place to handle this

@emmanuel Don't you think conformitas would be a great place to deal with mass-assignment security?

@solnic —
That's one of the major reasons I want to build conformitas: to externalize mass-assignment security responsibilities out of the models themselves. Filtering input to the models is a core responsibility of a form object (along with validating that input).
Handling input in a separate (form) object makes that filtering incredibly simple: a form object's list of attributes (with public writer methods) defines what input is acceptable. Eg., attributes with public writers replaces attr_accessible. Incidentally, the whole whitelist vs blacklist issue also disappears: blacklist-based filtering is no longer an option.

Oops, sounds like everyone here agrees that a form object is a win, I don't need to preach the benefits.
I should have read through the rest of the thread before responding :).

@emmanuel surprise! we're on the same page! ;)


Output added for Copy-DbaSqlDataCollector

Changes proposed in this pull request:

formatted help
Add message system
update camelCase
Add output object
All output changed to verbose
Add regex for one reference of source (fixes #1525 )

@potatoqualitee Do we need to rename this command to the actual object? They are not called Data Collector in SQL Server, it is Data Collection.
Was able to test. You will need to configure Data Collection on both source and destination, then just create the below collection set on your source. The original source of query below:

DECLARE @collection_set_id_1 INT
DECLARE @collection_set_uid_2 uniqueidentifier
EXEC [msdb].[dbo].[sp_syscollector_create_collection_set] @name=N'Cache Usage Report', @collection_mode=1,
@description=N'Cache Usage Report', @logging_level=1, @days_until_expiration=14,
@schedule_name=N'CollectorSchedule_Every_6h', @collection_set_id=@collection_set_id_1 OUTPUT,
@collection_set_uid=@collection_set_uid_2 OUTPUT

SELECT @collection_set_id_1, @collection_set_uid_2

DECLARE @collector_type_uid_3 uniqueidentifier
SELECT @collector_type_uid_3 = collector_type_uid FROM [msdb].[dbo].[syscollector_collector_types]
WHERE name = N'Generic T-SQL Query Collector Type';
DECLARE @collection_item_id_4 INT
EXEC [msdb].[dbo].[sp_syscollector_create_collection_item] @name=N'Cache Usage Report', @parameters=N'<ns:TSQLQueryCollector xmlns:ns="DataCollectorType"><Query><Value>

        SELECT objtype AS ''Cached Object Type'',
        COUNT(*) as ''Number of Plans'',
        SUM(cast(size_in_bytes as BIGINT))/1048576 as ''Plan Cache SIze (MB)'',
        avg(usecounts) as ''Avg Use Counts''
        from sys.dm_exec_cached_plans
        group by objtype
        order by objtype

        </Value><OutputTable>Cache_Usage_Report</OutputTable></Query></ns:TSQLQueryCollector>',
        @collection_item_id=@collection_item_id_4 OUTPUT, @frequency=5,
        @collection_set_id=@collection_set_id_1, @collector_type_uid=@collector_type_uid_3

SELECT @collection_item_id_4

you make a good point. Yes, let's rename. But I think we should also create like a... Invoke-DbaDataCollectionSetup or something?

I thought about it but had this feature gotten much love from MS? As well who really uses it?
I've had a few clients turn it on but we generally turn it back off because of noise with failures and such.

I don't know of anyone who uses it - that was my same experience. Noise.


improve maintenance mode api calls

IMPORTANT: Please do not create a Pull Request without creating an issue first.
Problem:

Our current automation tests use enableMaintenance api call to place a node in maintenance mode.
The api currently skips the check for maintenancePossible which is run by the UI as part of the enableMaintenance operation. This currently causes failures during tests.
Solution:

Minor change to enableMaintenance api call to run maintenancePossible before executing the actual maintenance operation.


Update link for API Keys page

Description of the change:
Reason for the change:
Link to original source:

Closes #

 Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.

Hello @DmytroLitvinov,Thanks again for the PR! We want to show our appreciation by sending                                              you some swag. Could you please fill out                                              this form so we can send it to you? Thanks!                                             Team SendGrid DX

Wow.
Thanks!
Great to hear it ;)


fix: close Postgres rows

Codecov Report
Base: 83.77% // Head: 83.71% // Decreases project coverage by -0.05% :warning:

Coverage data is based on head (9849ac0) compared to base (90fc814).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #369      +/-   ##
==========================================
- Coverage   83.77%   83.71%   -0.06%
==========================================
  Files          85       85
  Lines       13910    13918       +8
==========================================
- Hits        11653    11652       -1
- Misses       1878     1884       +6
- Partials      379      382       +3

Impacted Files
Coverage Δ

storage/postgres/postgres.go
67.62% <100.00%> (-0.08%)
:arrow_down:

server/commands/check_utils.go
94.60% <0.00%> (-1.25%)
:arrow_down:

server/commands/check.go
85.56% <0.00%> (-0.78%)
:arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

In each of these cases we iterate over all of the rows and according to the documentation

// Close closes the Rows, preventing further enumeration. If Next is called
// and returns false and there are no further result sets,
// the Rows are closed automatically and it will suffice to check the
// result of Err.

so explicitly calling Close is unnecessary, but I don't mind being explicit. If we do add these should we also add them to the MySQL implementation as well?


DataManagementApi - deleteById Method for Backing Stores

Feature #118
Upstream https://github.com/eclipse-dataspaceconnector/DataSpaceConnector/issues/801 and https://github.com/eclipse-dataspaceconnector/DataSpaceConnector/issues/802
Description
Implementation according to upstream issues 801 and 802.
Acceptance Criteria

[ ] AssetLoader Asset deleteById(String assetId) method is implemented
[ ] ContractDefinitionLoader ContractDefinition deleteById(String definitionId) is implemented
[ ] Unit tests

closing per daily conversation


make objectives use entityCategory

About the PR
BaseObjective has hideSpawnMenu and new objectives category for future use
Why / Balance
copy pasting noSpawn: true to every single objective bad
Technical details
has a test like action refactor pr
Media

[X] I have added screenshots/videos to this PR showcasing its changes ingame, or this PR does not require an ingame showcase

Breaking changes
any objective entities that didnt inherit from BaseObjective now must to not fail tests, and noSpawn: true can be removed
Changelog
no cl no fun

This should probably wait for space-wizards/RobustToolbox/pull/5061 and then just have it auto-categorize anything with the objectives component. I just need to figure out what was going wrong with tests.

yeah [EntityCategory("objectives")] would be nicer than copy pasting a test


Consider  github.com/webpack-contrib Org move

html-webpack-plugin is a very popular webpack plugin, and moving under the webpack-contrib org might be a good idea.

Would prefer to discuss this in the official webpack developer slack