looping trying to add package

Hello, I had an error that was similar to the one mentioned here.
However after running  go get -u ./... ,  I get following error 'go: github.com/facebookresearch/Clinical-Trial-Parser/src/cmd/cfg: looping trying to add package',

Does go get ./... give the same error?
I updated my Go to version go1.14.4, but it does not reproduce your problem.

Yes it gives same error message.
I just noticed that perhaps other materials were also missing
This below is the entire error message.
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/common/util/slice
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/common/resource
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/ct/parser
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/vocabularies
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/vocabularies/taxonomy
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/common/util/timer
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/common/lsh
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/common/util/intmath
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/ct/units
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/common/util/fio
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/ct/variables
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/common/col/set
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/common/param
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/cmd/search
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/common/col/tuple
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/ct/studies
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/vocabularies/mesh
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/ct/eligibility
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/ct/parser
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/vocabularies/umls
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/common/util/text
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/cmd/nel
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/common/trie
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/ct/parser/production
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/ct/criteria
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/common/conf
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/ct/relation
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/cmd/extract
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/cmd/cfg
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/ct/eligibility
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/common/param
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/common/util/timer
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/vocabularies/taxonomy
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/common/lsh
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/common/col/set
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/cmd/extract
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/ct/units
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/common/conf
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/vocabularies/umls
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/cmd/search
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/common/col/tuple
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/ct/parser
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/ct/variables
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/cmd/cfg
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/common/resource
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/ct/parser/production
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/common/trie
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/vocabularies
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/common/util/intmath
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/cmd/nel
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/ct/relation
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/ct/criteria
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/common/util/fio
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/vocabularies/mesh
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/common/util/slice
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/common/util/text
go: finding module for package github.com/facebookresearch/Clinical-Trial-Parser/src/ct/studies
go: github.com/facebookresearch/Clinical-Trial-Parser/src/cmd/cfg: looping trying to add package

I assume  go buid ./... etc. won't work either. Are you able to run any other go programs?
Go had the following issue: https://github.com/golang/go/issues/30394. I wonder if updating your Go version and cleaning up the packages would help. In addition, I can only suggest to delete the project, to clone it again, and to make sure that Go is setup properly.

I have installed the most recent version (1.14.4) as was in here, and followed the instructions here to set up.
I could run hello world example..
What do you mean by go setup properly?
Currently go is set up at ~/local/go/bin
cdsw@9tdvk9bwif7uyk7n:~/local/go$ which go
/home/cdsw/local/go/bin/go

and Clinical Trial Parser is at ~/local/go/src/github.com/facebookresearch/Clinical-Trial-Parser
cdsw@9tdvk9bwif7uyk7n:~/local/go/src/github.com/facebookresearch/Clinical-Trial-Parser$ ls
bin  CODE_OF_CONDUCT.md  CONTRIBUTING.md  data  doc  LICENSE  README.md  script  src

GOPATH is set up such that
cdsw@9tdvk9bwif7uyk7n:~/local/go/src/github.com/facebookresearch/Clinical-Trial-Parser$ printenv GOPATH
/home/cdsw/local/go

I just set things up on Monday and I don't know how I can do things differently.

That looks ok.  Would installing Go to /usr/local/go (=GOROOT) help?

The thing is I cannot install at /usr/local due to permission issue.
I have instead set the GOROOT also
cdsw@13bibixulsfuz8ec:~/local$ printenv GOROOT
/home/cdsw/local/go

However am getting the same error.

I can reproduce your problem when I install Go in the Go workspace. To fix this, you could, for example, set up your directories so that GOROOT=/home/cdsw/tools/go and GOPATH=/home/cdsw/workspaces/go (or /home/cdsw/local/...).  In addition, you may have to clean up /home/cdsw/local/go.

Thank you @salkola!  Issue got solved!! I will close the issue thank you for the help!


Issue with Same Or Before operator

The following expression passes CQL-to-ELM validation:
["Encounter, Performed": "URI Visit"] URIencounter
    with ["Diagnosis": "Upper Respiratory Infection"] URI
        such that URI.prevalencePeriod starts during URIencounter.relevantPeriod
        or (URI.prevalencePeriod starts same or before URIencounter.relevantPeriod
        and URI.prevalencePeriod overlaps URIencounter.relevantPeriod)

Note: This should fail per the signature for same or before in CQL R1.2, since the right-hand side for starts same or before is an interval and not datetime:
URI.prevalencePeriod starts same or before URIencounter.relevantPeriod

Same issue happens with same or after. Neither operator requires a datetime/time to be on the left- or right-hand side.
On the other hand, section <IP_ADDRESS> of CQL R1.2 seems to imply that same as or before can be used to compare two intervals, and that because of this, same or before or same or after could also be used to compare one interval with another. But if this is true, then the signatures for same or before or same or after do not appear to be completely documented in Appendix B.
In addition to the above information, there is a noted documentation issue with the same or before signature in Appendix B that was logged today: https://www.hl7.org/dstucomments/showdetail_comment.cfm?commentid=1426.
Thoughts?

The comment on the STU is a different issue, but still an errata in the spec. The issue you're highlighting here is that the spec isn't clear that "same or before" and "same or after" are the same operators as "on or before" and "on or after" when an interval is involved in the left or right, or both, operands.
So that's another comment to clarify in section 9.9.17 and 9.9.18 that "same" is a synonym for "on". I'll add that as a comment.

Logged this as comment 1433


ADD FOLDERS/SHORTCUTS TO FAVORITES

Simply add to favorites, be it folder or shortcuts. It can be through drag and drop or by clicking with the right mouse button "add to favorites".

Add to sidebar is what does that

You can add a folder to favorites by right click -> pin to sidebar. See #2026 for drag& drop support.


fixes #5

Apologies -- just spotted your PR after running into the exact same issue internally. Landed in #7 and will be released in version 0.2.3.


[Name renaming]

Describe the bug
When renaming or name for few seconds your url is not updating and you need to reload the page
To Reproduce
Change visible name to other and then click View Profile
Expected behavior
Profile
Screenshots

Browser
Google Chrome

OS: [Linux(Debian)]
Browser [chrome]
Version [128.0.6613.84 (Official Build) (64-bit)]

Additional context
Add any other context about the problem here.

Hi @lenixlenixlenix ,
Thanks for reaching out to us, we tried to reproduce this case without success, would you mind to confirm us if the issue still persist to you?
Many thanks in advance!


Panic on an Err value: InitialFileHeadersCorrupt

Sometimes, librespot panics with:
thread '<unnamed>' panicked at 'called `Result::unwrap()` on an `Err` value: InitialFileHeadersCorrupt', src/libcore/result.rs:799

This issue happens if a track is loaded in the Spotify Connect Client (so I can see the cover and the track artists and song title), but is not played locally (but was played before). Then I start librespot on my embedded Linux device, connect the client with librespot and after 3 or 4 seconds the panic happens.
Note, I am using Tremor as ogg decoder.

Not sure why OV_EINVAL is described with InitialFileHeadersCorrupt as vorbis-errors describe it as

OV_EINVAL       Invalid argument value.

Hm, I've just cross-compiled the current master for Raspberry Pi and always got InitialFileHeadersCorrupt, not yet found out the cause (not using Tremor btw)

Maybe this one helps: https://github.com/joerg-krause/librespot/commit/b58dc0b54e719101205dcd972062334cc72849db

I've cherry picked @joerg-krause's commit (thanks btw).
Since I can't reproduce this I'm not too sure how to fix the root cause.

Closed due to lack of maintenance of the project.


salmon quant for paired-end reads?

Hi,
The command for salmon quantification in this pipeline is specifying single-end reads:
salmon quant --threads $task.cpus --libType=U -i index -r $reads -o $pair_id
But the pipeline seems to take paired-end reads.
params.reads = "$baseDir/data/ggal/*_{1,2}.fq"
Can you clarify please, which is it?
From the salmon manual:
-r [ --unmatedReads ] arg   List of files containing unmated reads (e.g. single-end reads)
-1 [ --mates1 ] arg         File containing the #1 mates
-2 [ --mates2 ] arg         File containing the #2 mates

Thanks

You may be right, cc @skptic
On 23 Aug 2017 10:03 p.m., "Ashley S Doane"<EMAIL_ADDRESS>wrote:

Hi,
The command for salmon quantification in this pipeline is specifying
single-end reads:
salmon quant --threads $task.cpus --libType=U -i index -r $reads -o
$pair_id
But the pipeline seems to take paired-end reads.
params.reads = "$baseDir/data/ggal/*_{1,2}.fq"
Can you clarify please, which is it?
From the salmon manual:
-r [ --unmatedReads ] arg   List of files containing unmated reads (e.g. single-end reads)
-1 [ --mates1 ] arg         File containing the #1 mates
-2 [ --mates2 ] arg         File containing the #2 mates
Thanks
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/nextflow-io/rnaseq-nf/issues/1, or mute the thread
https://github.com/notifications/unsubscribe-auth/AAx3SJLr3rkLi7cBmC3KpSM_JzzLOr5Lks5sbJOpgaJpZM4PAlQR
.

I'm using a command like this, for paired fastq.gz files:
fr1=$1
fr2=$2
pair_id=$3

salmon quant -l A -i index --threads ${NSLOTS} -o $pair_id  -1 <(gzip -cd ${fr1}) -2 <(gzip -cd ${fr2}) --numBootstraps 30

The NSLOTS variable is just the cores I've specified in cluster options, and is exported by SGE into the environment on a compute node.  I put the above in a bash script, and call it from the nextflow main.nf.

Hi @DoaneAS ,
Thanks for pointing this out, you are indeed correct.
I have updated the repo with this commit which should solve the issue.

cheers, thanks


[BUG] Options support in @formio/angular

Environment
This issue regards the formBuilder in @formio/angular 5.2.2 and is not observed in angular-formio 4.11.5
When I pass options in angular-formio version, it works.
When I pass options in @formio/angular version, typescript screams about FormioOptions properties.
When I changed FormBuilderComponent.options from
options?: FormioOptions;
to
options?: any;
There is no error but still nothing happens.
Looking at FormioOptions definition TypeScript is correct as there are no properties that are defined in documentation and samples like editForm, builder, buttonSettings etc ...
How should I pass options to form builder?
Or @formio/angular is not for use and I should use angular-formio version?

Hosting type

[x ] Local deployment

Version:

Formio.js version: 4.14.7
Frontend framework: Angular 13
Browser: n/a
Browser version: n/a

Steps to Reproduce

define options like formioOptions = {editForm: { textfield: [{ key: 'api', ignore: true }] }}
pass to <form-builder [form]="form" [options]="formioOptions"></form-builder>

Expected behavior
Options are applied
Observed behavior
TypeScript error
error TS2559: Type '{ editForm: { textfield: { key: string; ignore: boolean; }[]; }; }' has no properties in common with type 'FormioOptions'.

We're currently addressing a backlog of GitHub issues, and as part of this effort, some inactive issues may be marked as closed. This isn't a dismissal, but a step toward more efficient tracking.
If you feel the issue is still relevant, please re-open and we'll ensure it gets the attention it deserves. Your understanding is appreciated as we work to enhance our open-source responsiveness.


(function(b){})(a) equivalent

As far as I can see there's no obvious way to emulate this in CS:
(function(d) {
    console.log(d.title);
})(document);

Would be cool to be able to, eg. with something like:
do (document as d) -> console.log d.title

Give it a try...
do (document) ->
  console.log document.title

Well the main idea was to have a shorter name for the reference, hence the as d. Nevermind, I'll survive without it ;)

do ->
  d = document
  console.log d.title

Sure. Thanks for your attention.

Were you simply looking for ((d)-> console.log d.title)(document)?

@michaelficarra nope, I was looking for doing something like:
do (document as d, foobar as f, anotherone as a, yetanotherone as y) ->
    console.log d, f, a, y

Instead of - as you suggest:
((d, f, h, a)->
    console.log d, f, a, y
)(document, foobar, anotherone, yetanotherone)

This syntaxic sugar being only matter of readability and concision, nothing more, nothing less.

$ coco -bcs
  let d = document
    console.log d.title

(function(d){
  console.log(d.title);
}.call(this, document));

I was looking for doing a loose augmentation module pattern in CoffeScript but I can not.
The pattern looks like this:
var MODULE = (function (my) {
  // add capabilities...

  return my;
}(MODULE || {}));

but in CoffeScript
MODULE = do (my) ->
  my

generates
var MODULE;

MODULE = (function(my) {
  return my;
})(my);

Support this syntax would be great!

Can I use the following syntax, even if it is less readable than using the keyword do:
MODULE = ((my) ->
  my
)(BAR or {})

Thx to @naholyr for the hint.

@piouPiouM: there's nothing stopping you

@michaelficarra arf, I made a typo ^^'

$ coffee -bep 'module = do (my = BAR or {}) -> my'
var module;

module = (function(my) {
  return my;
})(BAR || {});

@n1k0:
$ coffee -bep 'do (d = document) -> console.log d.title'
(function(d) {
  return console.log(d.title);
})(document);

@michaelficarra, I do not get the same result with version 1.2.0:
$ coffee --version
CoffeeScript version 1.2.0
$ coffee -bep 'module = do (my = BAR or {}) -> my'
var module;

module = (function(my) {
  if (my == null) my = BAR || {};
  return my;
})(my);

@michaelficarra @piouPiouM: Same here with 1.1.3 :/
$ coffee --version
CoffeeScript version 1.1.3
$ coffee -bep 'module = do (my = BAR or {}) -> my'
var module;

module = (function(my) {
  if (my == null) my = BAR || {};
  return my;
})(my);
$ coffee -bep 'do (d = document) -> console.log d.title'
(function(d) {
  if (d == null) d = document;
  return console.log(d.title);
})(d);

See Jeremy's comment and a477880edc9a46c4a47f8e1b6f2dcf6fc5fda6c7. Use master if you want that functionality or just wait for the next release.

@michaelficarra ahhh cool, that's awesome news :)


cmd/go: unknown revision when using semver tag for module in subdirectory

What version of Go are you using (go version)?

$ go version
go version go1.11.5 darwin/amd64

Does this issue reproduce with the latest release?
Yes. It also reproduces with go1.12beta2.
What operating system and processor architecture are you using (go env)?
go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/username/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/username/go"
GOPROXY=""
GORACE=""
GOROOT="/Users/username/Homebrew/Cellar/go/1.11.5/libexec"
GOTMPDIR=""
GOTOOLDIR="/Users/username/Homebrew/Cellar/go/1.11.5/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/v2/2k1mwftj6dv08m7kq2q9lbh40000gn/T/go-build582638908=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?
In a local go module, I tried to require a specific tagged version of a go module available from a subdirectory in a remote repo. go build returns an error giving unknown revision. When I remove the require and let it figure out the dependency automatically, it adds the module to the go.mod using a pseudo-version despite being able to use a tagged version when requiring a separate module that resides at the same remote repo's root. Here is a complete repro using github.com/burnson/gomoduletest as the remote repo. Step 1 creates the remote repo from scratch and Step 2 reproduces the pseudo-version behavior and build error when an explicit tag is used instead.
##
## STEP 1: create a remote repo with a module at the
##         repo root and a module in a subdirectory
##         (skip this if you want to repro with the
##         the GitHub repo provided at $TEST_REPO)
##

# A repo you have write access to
TEST_REPO=burnson/gomoduletest

# Do this in a scratch directory
cd $(mktemp -d)
echo "Creating repo from here:"
pwd

# Make a git repo
mkdir gomoduletest
cd gomoduletest
git init

# Add a module at the repo root
cat >go.mod <<EOF
module github.com/$TEST_REPO
EOF

# Create a simple package that returns the package name
cat >gomoduletest.go <<EOF
package gomoduletest

func Package() string {
	return "gomoduletest"
}
EOF

# Check the gomoduletest build
go build

# Now add a new module to a subdirectory
mkdir subpackage
cd subpackage
cat >go.mod <<EOF
module github.com/$TEST_REPO/subpackage
EOF

# Create a simple package that returns the package name
cat >subpackage.go <<EOF
package subpackage

func Package() string {
	return "subpackage"
}
EOF

# Check the subpackage build
go build

# What does everything look like so far?
cd ..
find *

# Commit what we have and tag as v1.0.0.
git add .
git commit -m 'v1.0.0 release'
git tag v1.0.0

# Push this repo to github
git remote add origin git@github.com:$TEST_REPO.git
git push -fu origin master
git push -f --tags

##
## STEP 2: create a local go module that makes use of
##         both of the modules created in the remote
##         repo above
##

# The repo created above
TEST_REPO=burnson/gomoduletest

# Do this in a new scratch directory
cd $(mktemp -d)
echo "Creating local go module from here:"
pwd
mkdir localmodule
cd localmodule

# Create a go module file for the test program
cat >go.mod <<EOF
module my/program
EOF

# Create a test program that uses both modules
# and calls the Package() func
cat >main.go <<EOF
package main

import (
	"fmt"

	rootpackage "github.com/$TEST_REPO"
	"github.com/$TEST_REPO/subpackage"
)

func main() {
	fmt.Println(rootpackage.Package())
	fmt.Println(subpackage.Package())
}
EOF

# Build the program
go build

echo
echo "Test program source:"
cat main.go

# Run the program
echo
echo "Test program output:"
./program

# Check the contents of the go.mod file now:
echo
echo "Test program go.mod:"
cat go.mod

# Replace the pseudo-version with the v1.0.0 tag:
cat >go.mod <<EOF
module my/program

require (
	github.com/$TEST_REPO v1.0.0
	github.com/$TEST_REPO/subpackage v1.0.0
)
EOF

echo
echo "Edited go.mod forcing v1.0.0 for subpackage:"
cat go.mod

# Try building
echo
echo "Building with tagged version:"
go build

What did you expect to see?
In the local module test, I expected to see go build tag subpackage with the v1.0.0 semver tag I pushed to the remote repo in go.mod. Furthermore, when I edit go.mod to explicitly use v1.0.0, I expect it to find the revision when I go build.
What did you see instead?
go build finds the subpackage module but writes it using a pseudo-version in go.mod. If I override that with the tagged version v1.0.0, I get an unknown revision error. However, the revision is detected for the module that resides at the remote repo root, which is a contradiction.
...
Test program source:
package main

import (
	"fmt"

	rootpackage "github.com/burnson/gomoduletest"
	"github.com/burnson/gomoduletest/subpackage"
)

func main() {
	fmt.Println(rootpackage.Package())
	fmt.Println(subpackage.Package())
}

Test program output:
gomoduletest
subpackage

Test program go.mod:
module my/program

require (
	github.com/burnson/gomoduletest v1.0.0
	github.com/burnson/gomoduletest/subpackage v0.0.0-20190326022552-ee56fd37aed9
)

Edited go.mod forcing v1.0.0 for subpackage:
module my/program

require (
	github.com/burnson/gomoduletest v1.0.0
	github.com/burnson/gomoduletest/subpackage v1.0.0
)

Building with tagged version:
go: finding github.com/burnson/gomoduletest/subpackage v1.0.0
go<EMAIL_ADDRESS>unknown revision subpackage/v1.0.0
go: error loading module requirements

It looks like the tag prefix is missing for the nested module. Each module in a repository has a separate namespace of versions, distinguished by a subdirectory prefix. So for the module, github.com/burnson/gomoduletest/subpackage in the repository github.com/burnson/gomoduletest, the prefix should be subpackage/, and the full tag should be subpackage/v1.0.0. The tag v1.0.0 without a prefix is a version for the module at the root of the repository.
This behavior is described on the Modules wiki, which is probably the best source of information on this subject. I don't believe go help modules actually describes this, so I'll mark this as a documentation bug.

On second thought, go help modules may not be the best place for this. It says very little about repositories and publishing releases.
I've added a section to the wiki, Publishing a Release that gives more specific instructions.

Ah, this is what I overlooked from my initial readthrough of the Modules wiki:

Version tags for modules below the root of the repository must include the relative directory as a prefix.

Thanks for the additional clarification on the wiki.


Add support for forcing cargo:rustc-link-lib to link as native

This adds support for setting SQLITE_STATIC=1 (or SQLCIPHER_STATIC for sqlcipher builds) in the environment to force us to link sqlite/sqlcipher statically. If this isn't specified, or it's specified to be 0, it uses the default which is dylib.
This is needed* in order to build a statically linked rust library that includes these (without the bundled feature).
This is still not perfect, a more robust build.rs might inspect what the actual library files are and decide based on that, as the openssl-sys crate does https://github.com/sfackler/rust-openssl/blob/91a78bc38b09b7e2dd2a0b493407e3fb0550072a/openssl-sys/build/main.rs#L572, but that's complex and doesn't seem super worthwhile.
* Well, "needed".

Thanks.


Evaluator Review: Developer Handbook Milestone 1

Review, approve or deny https://github.com/eosnetworkfoundation/grant-milestones/pull/41

Approved, progress appears to correspond expectations for milestone 1


"mbed update" does not update mbed-os

It appears that "mbed update" does not actually update the mbed-os library - it changes the hash in the mbed-os.lib file but does not checkout the corresponding version.
jac-16:~/mbed/bls-test $ mbed --version
0.9.7
jac-16:~/mbed/bls-test $ mbed ls
bls-test (bda57f050ffd)
|- bls (a0caf94e9d38)
`- mbed-os (21dd7008a154)
jac-16:~/mbed/bls-test $ mbed update --clean --clean-deps
[mbed] Synchronizing dependency references...
[mbed] Updating program "bls-test" to latest revision in the current branch
[mbed] Updating library "mbed-os" to rev #a6f3fd1a60d5
[mbed] Updating library "bls" to rev #a0caf94e9d38
jac-16:~/mbed/bls-test $
jac-16:~/mbed/bls-test $ mbed ls
bls-test (bda57f050ffd)
|- bls (a0caf94e9d38)
`- mbed-os (21dd7008a154)

@infinnovation can you provide me with an example that I can reproduce locally? Perhaps the bls-test program (without the source code)?

I've set both bls-test and bls as Public (Unlisted) to allow you to fetch them.  And in testing that, I have a simpler anomaly:
jac-16:~/mbed $ mbed import https://developer.mbed.org/users/infinnovation/code/bls-test
[mbed] Importing program "bls-test" from "https://developer.mbed.org/users/infinnovation/code/bls-test" at latest revision in the current branch
[mbed] Adding library "mbed-os" from "https://github.com/ARMmbed/mbed-os" at rev #a6f3fd1a60d5
[mbed] Adding library "bls" from "https://developer.mbed.org/users/infinnovation/code/bls" at rev #a0caf94e9d38
jac-16:~/mbed $
jac-16:~/mbed $ cd bls-test
jac-16:~/mbed/bls-test $ mbed ls
bls-test (bda57f050ffd)
|- bls (a0caf94e9d38)
`- mbed-os (21dd7008a154)

Looks like it's something to do with the cache.  I removed cache/github.com/ARMmbed/mbed-os and tried again; this time it got the proper version of mbed-os

@infinnovation Have you enabled the experimental caching in mbed CLI?

Yes, I have enabled caching, and I was aware that it is experimental.  Perhaps the two test-cases I've inadvertently generated will help its advancement towards fully working :-)
The .git repo in the cache (before I removed it) seemed to have been fully "fetched" (including tag mbed-os-5.1.5) but HEAD was at 21dd700...

@infinnovation PR #374 is aiming to fix this. Think you could try it?

@screamerbg I've tried it out and it works fine.

Now merged and part of the mbed CLI 0.9.10 release. Please reopen if the issue persists.


ICA and line noise can be unstable?

I have been trying to remove the line noise of some ECoG data using ICA (I am trying to avoid bandstop filter here).
However, the results I get are pretty unstable, and I am not quite sure why. That is, fitting the ICA several times of the same data can give radically different results (very good or very bad).

I think I checked that this was not a copy/memory problem.
Setting the ICA(random_state) stabilizes the results (but is arbitrary, since different random_state give very different results).

Here is a summary of what I do [complete notebook here]:

sources = 1/F random source signal
mix these source signals together with a line noise (raw = dot(cov, vstack(sources, linenoise))
filtered_raw = raw.copy().filter(55, 65) to filter around line noise (because of this ref)
ICA().fit(filtered_raw)`
compute PSD of the data in component space dot(ica.components, original_raw)
find the component carying the line noise: bad
remove it from the data: ica.apply(original_raw, exclude=[bad])
check the PSD of this cleaned data

This works ~ well for low sampling frequency with or without decim>1, but gets unstable for high sampling frequency when we use a decim>1.
This could be due to aliasing, so I tried to shuffle the data just prior to fitting the ICA
times = np.arange(raw.n_times)
np.random.shuffle(times)
raw._data = raw._data[:, times]

the results are a bit better, but still pretty unstable.  In the notebook, you can see that it works 50% of the time
Any idea what I do wrong?

Are you using method= ‘extended-infomax’? If I remember correctly this method should be better in catching line noise components.

And just by the way: do you think implementing eeglab's cleanline in mne would be a worthwhile investment?

I like cleanline, although when I was using it, it had a really high crash propensity.
It has a bunch of advantages over Ica.

@mmagnuski extended-infomax seems more stable indeed, thanks!
Note that when using decim>1, we need to shuffle time.
Just had a quick look at cleanline, seems like a more robust approach in principle. Anyone willing to implement it in python?...

Not me :)
Extended infomax was especially developed to deal with components like line noise which are non Gaussian.
At this point, my vote would be for deprecating standard infomax.

my vote would be for deprecating standard infomax

+1 from me, or at least actually using extended as the default as it sounds like we advertise it that way. Okay with you @dengemann?

Just had a quick look at cleanline, seems like a more robust approach in principle

Can you describe briefly what it does?
Have you tried "spectrum_fit" mode?
http://mne-tools.github.io/stable/generated/mne.io.RawFIF.html#mne.io.RawFIF.notch_filter

@Eric89GXL I'm too lazy to check right now, but in theory, it should always be clear if you're using extended or not because you call infomax via method='infomax' or method='extended-infomax'.
But it might be preferable to have both link to extended-infomax and only allow turning off extended manually.

Okay with you @dengemann?

+0 ... mabye someone needs it for research questions.  No pressure to deprecate it.

You can still set it via fit_params right?

@dengemann even without deprecation of non-extended mode, do you agree extended should be on by default? It sounds like the default is non-extended even though extended is supposed to be the default based on the docs.

Just had a quick look at cleanline, seems like a more robust approach in principle

Can you describe briefly what it does?

It fits sinusoids at the expected line noise (and harmonics) to each channel and subtracts the best.

do you agree extended should be on by default?

no, our default is fastica and it's the ICA method preferred by me and Alex.

It fits sinusoids at the expected line noise (and harmonics) to each channel and subtracts the best.

The best what?
If it's doing an inner product and subtracting out the component, it sounds like it should be less robust than using "spectrum_fit" (which uses multitaper methods). Also, filter_chpi does this sort of thing (fits individual sisusoids) in a time-varying way, so you could use that if you want -- we'd just need to allow a mode that actually ignores cHPI signals, or generalize it to a different function name for this purpose.

well maybe it is ok to kind of change the order of the documentation and name infomax-extended first and suggest to use that if fastica is not used.

@Eric89GXL I was wrong, but see:

Many analysts automatically perform a notch filter at 60 Hz to remove line noise. Such notch filters often use a notch width of 10 Hz or larger, resulting in significant signal distortion in frequencies between 50 and 70 Hz. This distortion should not be an issue for analyses that immediately apply a low-pass filter to the signal, say at 40 Hz, but may preclude certain high-frequency studies. Some studies have also shown that non-causal low-pass filtering can significantly change ERP onsets (Vanrullen, 2011; Rousselet, 2012). In addition, Barnett and Seth (2011) have shown that filtering, particularly low pass filtering, can have a damaging impact on Granger causality and other connectivity computations.

Mitra and Pesaran (1999) suggest a multi-taper decomposition approach for identifying and removing line noise components while minimizing background signal distortion. We leverage routines from the cleanline EEGLAB plugin developed by Mullen (2012), which extends functionality from the open source Chronux toolbox (Mitra and Bokil, 2007). This method traverses the data using a short sliding window (4 s with a 1-s slide by default). The method transforms the data in each window into the frequency domain using a set of Slepian or multi-tapers with a predetermined spectral resolution. Such tapers are ideal for isolating spectral energy within frequency bands, even for short time windows. By default, we use a taper bandwidth (TBW) of 2 Hz in each 4-s sliding window (W), which contains N sample points. The Slepian tapers are created by calling dpss(N, TBWW/2, TBWW–1) where dpss is the discrete prolate spherical sequence function from the MATLAB Signal Processing Toolbox.

The cleanline method fits a frequency-domain regression model to estimate the amplitude and phase of a deterministic sinusoid of a specified frequency, embedded in locally white noise. This is an idealized model of sinusoidal line noise of unknown phase and amplitude. A Thompson F-test assesses whether the complex amplitude is significantly non-zero. If the amplitude is significant (p < 0.01), the method reconstructs the time-domain sinusoid for the line noise frequencies. The method stitches together results from successive overlapping windows by using a sigmoidal weighted average specified by a smoothing parameter tau (100 by default). Finally, the method subtracts this fitted signal from the data. We repeat this process (a maximum of 10 iterations by default) until the sinusoid amplitude for the specified frequencies is not significantly different from the background.

In practice, the exact line frequency is unknown and variable. Following Mitra and Pesaran, we apply the regression model across a range of frequencies centered on each candidate line-noise frequency and select the frequency that maximizes the Thompson F-statistic. The range is +/− fScanBandwidth, which is 2 Hz by default. The user must provide a rough estimation of the line frequencies or use the defaults of 60 Hz with harmonics that are multiples of 60 up to the Nyquist frequency. The advantage of this approach over notch filtering is that it removes only deterministic line components, while preserving “background” spectral energy. The sliding window estimation further allows for non-stationarities in the phase and amplitude of the line component. The PREP pipeline function encapsulates this denoising functionality in the function cleanLineNoise.

http://www.ncbi.nlm.nih.gov/pmc/articles/PMC4471356/

Warning, this may be dgwakeman noise, but
Most line noise is not a pure sinusoid, so it ends up being a different signal from the cHPI in that respect

@Eric89GXL I don't clearly understand spectrum_fit, an example would welp.
@jona-sassenhagen With regard to default changes, I think it is more critical to have a adapt or write clear tutorials on when to use each ICA method, rather than changing the defaults of ICA as ICA can be used in different contexts and for different purposes.

Most line noise is not a pure sinusoid

+1, and it's definitely not stationary.

I don't clearly understand spectrum_fit, an example would welp.
Mitra and Pesaran (1999) suggest a multi-taper decomposition approach

This part at least is what "spectrum_fit" is [meant to implement] (https://github.com/mne-tools/mne-python/blob/master/mne/filter.py#L1072). So you could look at the ref @jona-sassenhagen mentioned to see the relationships. I guess you could extend it to the "cleanline" approach if you want.

@jona-sassenhagen With regard to default changes, I think it is more critical to have a adapt or write clear tutorials on when to use each ICA method, rather than changing the defaults of ICA as ICA can be used in different contexts and for different purposes.

I've told Denis a few times I'm really impressed with MNE infomax, but IMO it's a mistake to make it so easy to use non-extended infomax. It's basically outdated, as far as I can tell. You ran into this issue because you assumed method='infomax' was a reasonable choice, which it isn't.
So basically, here is your tutorial: use 'fastica' or 'extended-infomax', depending on if you have the time to fix infomax and if you want your components ordered by explained variance. Don't use 'infomax'.

Most line noise is not a pure sinusoid, so it ends up being a different signal from the cHPI in that respect

That is a good point -- it is not stationary, plus it has lots of harmonics. FYI cHPI is also not stationary, though -- head movements act as amplitude modulators on the cHPI signals, thereby spreading activation. That's why a time-varying fit is used in filter_chpi (which does the same thing as MaxFilter for cHPI artifact removal).

@Eric89GXL If I understand correctly (else excuse for the discussion noise), spectrum_fit is used to notch out sinusoidal frequencies in each channel separately, isn't it? Whereas the ICA is multivariate: it aims at finding the common component that capture the line noise. In theory, there's no need to filter line noise/sinusoids once you regressed out the line component from the data.

You're losing rank though, and it assumes perfect stationarity and a few other things.
It's good, but not perfect - if I was looking into the frequencies around line noise, I'd probably be concerned.

... Whereas the ICA is multivariate ...

I haven't looked into these much, but that sounds right to me.

You're losing rank though

You're loosing rank, but in EEG it makes sense. One rank has to be devoted to the line, no?

it assumes perfect stationarity and a few other things.

I don't think it assumes frequency stationarity. The line noise could be disrupted, as long as the noise component is common across electrodes, the ICA will capture it. In fact, the notebook I shuffle time (and thus frequency stationariy) to make ICA more robust.

I mean spatially.
And with other methods you don't necessarily lose rank.

@kingjr
Do you need to shuffle time? Eeglab Infomax shuffles time by default (I'm not sure how often) but in general the order of samples shouldn't matter so much as components are assumed to be stationary.
With respect to cleanline vs ica: I've had files where ICA removed line noise without much problem, but in some cases it was helpless. But I've had problems with cleanline too - I usually resort cleanline when ICA doesn't help.
@Eric89GXL
So it seems cleanline is very close to spectrum_fit. Cleanline is performed in time windows and the estimated parameters are then smoothed lineary or nonlineary (there's a parameter for that) across windows. This can be problematic at times btw.

I mean spatially.

Ok. Is it common to have spatially varying line noise?

And with other methods you don't necessarily lose rank.

That makes me wonder whether we shouldn't actually keep bad ECoG (e.g. in white matter) to preserve a high rank (similar for loose EEG?).

Do you need to shuffle time?

If I use the decim of ICA, yes (see here), else you introduce aliasing before the ICA fit. But yes, ICA is blind to time.

FYI the default EEGLAB solver is infomax

With extended 1 though.

Should we close this, or is there some code action to be taken? Maybe updating the ICA docs somewhere to mention that line noise can be challenging etc.?

I'm closing it for now. Thank you all for your comments and help.


Create test

fff

Collaborator: Review was completed but status check for commit e9f5914 was not changed as it is HEAD for open pull request #19

Corresponding review in Collaborator has been completed.Author: Administrator.Reviewer: test1.Observer: test2.

Collaborator: Review was completed but status check for commit 90a8949 was not changed as it is HEAD for open pull request #20

Corresponding review in Collaborator has been completed.Author: Administrator.Reviewer: test1.Observer: test2.

Collaborator: Review was completed but status check for commit 1cf94e4 was not changed as it is HEAD for open pull request #22

Corresponding review in Collaborator has been completed.Author: Administrator.Reviewer: test1.Observer: test2.

Collaborator: Review was rejected but status check for commit 37f84e7 was not changed as it is HEAD for open pull request #27

Corresponding review in Collaborator has been rejected.Rejected by admin


🌿 Add TypeScript SDK generator

Adds a new Fern TypeScript Node SDK generator and the NPM_TOKEN environment variable to the fern-generate job in the ci workflow.

@kafkas merged! 🙏


[Image][Performance] Fast Image with Auth Token Header Caching

Details

This PR combines the draft PR from @Szymon20000 https://github.com/Expensify/App/pull/11009 for adding fast image and the PoC auth token PR from @Beamanator https://github.com/Expensify/App/pull/12146 to properly cache images using fast image with the auth token being passed in the headers rather than the URL.
Fixed Issues

https://github.com/Expensify/App/issues/10792
$ GH_LINK
PROPOSAL: GH_LINK_ISSUE(COMMENT)
Tests

Checkout this branch
Run the app on both iOS and Android
Open a chat with some image attachments
They will load in as normal the first time, but reload the app and then you will see the cached images loaded instantly in the same chat

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

QA Steps

[ ] Open a report chat and add a new image attachment and send so the image appears in the chat (this should display a loading indicator the first time it is requested)
[ ] Reload the app and you should see the cached image displayed (with no loading indicator)
[ ] Click on an image in the chat to open the attachment modal view, close it and reopen it to see the cached image displayed without a loading state
[ ] Verify that any attachments still download as normal without any errors (including image attachments and PDFs)

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 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 included screenshots or videos for tests on all platforms
[x] I ran the tests on all platforms & verified they passed on:

[x] iOS / native
[x] Android / native
[x] iOS / Safari
[x] Android / Chrome
[x] MacOS / Chrome
[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 was added in all src/languages/* files
[x] I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[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] 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 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(themeColors.componentBG)

[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] I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

PR Reviewer Checklist
The reviewer will copy/paste it into a new comment and complete it after the author checklist is completed

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

[ ] I verified the steps for local testing are in the Tests section
[ ] I verified the steps for Staging and/or Production testing are in the QA steps section
[ ] 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)
[ ] 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)

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

[ ] iOS / native
[ ] Android / native
[ ] iOS / Safari
[ ] Android / Chrome
[ ] MacOS / Chrome
[ ] MacOS / Desktop

[ ] 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
[ ] I verified proper code patterns were followed (see Reviewing the code)

[ ] 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).
[ ] I verified that comments were added to code that is not self explanatory
[ ] 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.
[ ] I verified any copy / text shown in the product was added in all src/languages/* files
[ ] I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[ ] 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.
[ ] I verified the JSDocs style guidelines (in STYLE.md) were followed

[ ] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[ ] I verified that this PR follows the guidelines as stated in the Review Guidelines
[ ] 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)
[ ] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[ ] 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
[ ] If a new component is created I verified that:

[ ] A similar component doesn't exist in the codebase
[ ] All props are defined accurately and each prop has a /** comment above it */
[ ] The file is named correctly
[ ] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[ ] The only data being stored in the state is data necessary for rendering and nothing else
[ ] 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)
[ ] 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)
[ ] All JSX used for rendering exists in the render method
[ ] 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

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

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

[ ] 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)
[ ] 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.
[ ] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots

Web

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

I have read the CLA Document and I hereby sign the CLA

@thomas-coldwell can you retroactively sign your commits? please
Asked here about it: https://github.com/Expensify/App/issues/10792#issuecomment-1311084477

Will get those sorted now @aldo-expensify cheers!

Looks like we need to have a think about how this should work on Desktop and Web (inc. mWeb) before this PR is merged cc @Beamanator

Yo @thomas-coldwell so correct me if I'm wrong but I had two assumptions in regard to web / desktop:

I thought react-native-fast-image wouldn't work well for Web / Desktop, is that incorrect? For some reason I thought there was some bug that prevent us from using that lib on web / desktop
Iff there's a problem using react-native-fast-image on web i think that's fine, because we have this issue (https://github.com/Expensify/App/issues/12603) that should fix a bug in react-native-web which will allow us to pass the auth token in an Image's header on web / deskotp

Hey @Beamanator you are correct - there is no implementation of react-native-fast-image on web so we'll have to add that ourselves. I think if we add web support to fast-image (subject to that bug fix for the header caching on RNW) then that keeps it clean and the main app can then use fast-image and has the platform implementation details hidden when its used in the Expensify app and then this PR should start working perfectly (will test and provide screenshots of those missing platforms once that support is added). Does this sound good?

Just seen @kidroca 's comment here which matches up with my previous comment 😄

Sorry I am going OOO, so I won't be able to take this. Please re-assign.

@mananjadhav you were assigned to this issue as C+ here: https://github.com/Expensify/App/issues/10792
Mind helping review this?

Sorry just getting to your comment @thomas-coldwell 😅
So it sounds like you think it could be best for us to implement a way for FastImage to work in Web, and hold this PR on that? As I mentioned in this comment I thinkkk it would be nice to not to have to wait for an update to FastImage (making it work for Web) since this could take some time, and instead we could start with implementing some Image component in our app that uses react-native's Image on web / desktop & uses react-native-fast-image on native devices?
Note: This may be as simple as:

index.native.js -> <FastImage {...props} />
index.js -> <FastImage fallback {...props} />

(as proposed by @kidroca )

@mananjadhav no worries! Actually please hold off testing this till we're settled on exactly the plan for this PR! 🙏 Thanks!

Yea I'm in agreement with you @Beamanator that we have an Image component in the app that uses fast image on native platforms and then RNW's Image component on web and desktop and then eventually (in another PR down the road) move this into the fast image fork 🙌

Sounds great @thomas-coldwell !

Oh one point @thomas-coldwell : I thinkkk we can actuallly work in parallel, here's how i'm thinking:

In Native, we'd use react-native-fast-image (as you mentioned)
In Web, eventually we'll pass headers (once @kidroca 's fix is ready) but for now we can use the old / current way (passing the encrypted auth token in the URL)

This way, we don't have to wait on anyone, right? I think y'all can both proceed w/ PRs separately, then:

If @kidroca gets his PR merged before this one, we can implement the web fix in this PR
If this PR gets merged first, we can implement the react-native-web fix in @kidroca's PR

Okay yeah that sounds perfect - good point! I'll get this sorted first thing tomorrow so we can get the PR reviewed and merged ASAP! @Beamanator

Awesome thanks @thomas-coldwell ! Looking forward to reviewing tomorrow 💪

The fallback prop doesn't seem to work as react-native-fast-image has a single entry point file that calls requireNativeComponent which throws an error as this is obviously a native specific method - it appears someone did try to create a PR to support this but its still open https://github.com/DylanVann/react-native-fast-image/pull/481
I've already setup the separate index.js and index.native.js files for a FastImage wrapper component in the app, but will just modify the index.js to directly use RNW's Image component with the encryptedAuthToken in the url if that sounds good @Beamanator ?

I've tested this and it works on all platforms now (updated screenshots to reflect this) - if you are happy with the setup I mentioned in my previous comment @Beamanator then I think this should be ready for review 🙂

Thanks for the investigation @thomas-coldwell 👍 Seems we can push that part (using react-native-fast-image everywhere) to a future task (not so high priority) and review what you've got so far!
@mananjadhav are you able to review this PR today? We'd like to move this forward kinda quickly. Sadly I'll be OOO most of today through Monday, so @aldo-expensify do you mind working with @mananjadhav to get this tested & hopefully merged relatively soon? :D 🙏

I'll be able to move this along today second half of my day

@thomas-coldwell I've raised one or two questions on the PR. Apart from that I am facing an issue that the Attachment View doesn't load in the mweb safari and mweb chrome.
https://user-images.githubusercontent.com/3069065/202513223-b933f5e9-ed9d-45f1-82e0-154177dd945d.mo
https://user-images.githubusercontent.com/3069065/202513259-fe361ab3-bd16-4137-9baa-42ffd5b3473e.mov
This works fine in main branch.

Next question I had was, in web any subsequent attempt of opening the attachment is fast with immediate load. But the same shows a spinner on every subsequent view. Is that expected?

Hey @mananjadhav thank you for the review - you caught a big area I'd overlooked during testing with the attachment modal. I've just pushed some changes that address the following:

The images in the attachment view were having the encrypted auth token added twice to the source URL, once by the attachment modal component and then secondly by its child ImageView component. This resulted in an invalid URL and hence nothing was displayed. I've tweaked this so instead the isAuthTokenRequired prop is passed down to the ImageView component which ultimately handles applying either the headers or adding an encrypted auth token URL param. Additionally, this fixes the native (iOS and Android) image caching in the attachments modal as it now uses the auth token in the headers
The FastImage wrapper also now uses the onLoad method to return the width and height of the image rather than using Image.getSize on all platforms which would result in a subsequent, uncached request

Thanks for the changes @thomas-coldwell. I'll get to the second round in a few hours.

@thomas-coldwell The images load in the preview now. I also ran the regression on https://github.com/Expensify/App/issues/8849 and https://github.com/Expensify/App/issues/5193.
I still found two issues:

On iOS if I load a smaller image, it flickers because  the dimensions change

https://user-images.githubusercontent.com/3069065/202889567-6bb93a6a-e26e-4514-ab46-327608549c89.mov

On Android, 5193 is breaking, long image previews look blurry.

https://user-images.githubusercontent.com/3069065/202889489-86231460-1a05-4b9f-879a-f0fe5fc5890b.mov

This should fix the issue with the layout shift on all native platforms. It now hides the FastImage until it has got the actual image width and height required for the ImageZoom component.
Ideally I would have left this using Image.getSize but this results in an uncached request on each open of the attachment modal and thus a loading state. Furthermore, the only way to get the image width and height using FastImage is from the onLoad method - there is sadly no static FastImage.getSize method, though this might be something to look at in a future PR cc @Beamanator ?

Ideally I would have left this using Image.getSize but this results in an uncached request on each open of the attachment modal and thus a loading state. Furthermore, the only way to get the image width and height using FastImage is from the onLoad method - there is sadly no static FastImage.getSize method, though this might be something to look at in a future PR cc @Beamanator ?

Image.getSize does not work with a ImageSource object, but just the uri, so it won't work for requests with headers
https://reactnative.dev/docs/image#getsize
Image.getSize(uri, success, [failure]);

Image.getSize downloads the image in order to get it size, so it's virtually like using the onLoad event
(I've also made the onLoad event for react-native-web Image return the same width and height that the getSize returns)
IMO we should use onLoad event for the time being and if we really need getSize (why?) we can submit PRs to update the interface(s) to support an ImageSource input and not just the uri string

@mananjadhav I think we're good to move forward with the current approach mentioned by @kidroca , to use FastImage's onload - do you have any concerns? If not, let's keep this moving forward!

I've converted that over to a class component now - should be good for a final review @mananjadhav 🙌

Thanks @thomas-coldwell ! Looking forward to hearing how testing goes @mananjadhav ! 💪

Thanks for the patience here @thomas-coldwell @Beamanator. Appreciate the quick turnarounds for the changes. I've tested and it works great! Attaching the screencasts.
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] iOS / native
[x] Android / native
[x] iOS / Safari
[x] Android / Chrome
[x] MacOS / Chrome
[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 was added in all src/languages/* files
[x] I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[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 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(themeColors.componentBG)

[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] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots

Web
https://user-images.githubusercontent.com/3069065/203362424-8023d210-ce73-41ae-a3b1-2a1aa008de0f.mov
Mobile Web - Chrome

https://user-images.githubusercontent.com/3069065/203362659-ec199b38-4a76-4bde-81a7-dea28ba38220.mov
Mobile Web - Safari

https://user-images.githubusercontent.com/3069065/203362710-88d4a7ff-4bbe-43ba-a419-b196e5a71e30.mov
Desktop

https://user-images.githubusercontent.com/3069065/203362799-56901600-f67c-4cd8-9623-216ed2c6b32e.mov
iOS

https://user-images.githubusercontent.com/3069065/203362949-62ba7272-e5dc-49fd-97ed-164945859884.mov
Android

https://user-images.githubusercontent.com/3069065/203363023-4ab7c79e-9263-4f4f-b731-8a26c3ab98d5.mov

Remove the line @mananjadhav :)

@Beamanator @aldo-expensify All yours.

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] iOS / native
[x] Android / native
[x] iOS / Safari
[x] Android / Chrome
[x] MacOS / Chrome
[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 was added in all src/languages/* files
[x] I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[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 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(themeColors.componentBG)

[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] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots

Web
https://user-images.githubusercontent.com/3069065/203362424-8023d210-ce73-41ae-a3b1-2a1aa008de0f.mov
Mobile Web - Chrome

https://user-images.githubusercontent.com/3069065/203362659-ec199b38-4a76-4bde-81a7-dea28ba38220.mov
Mobile Web - Safari

https://user-images.githubusercontent.com/3069065/203362710-88d4a7ff-4bbe-43ba-a419-b196e5a71e30.mov
Desktop

https://user-images.githubusercontent.com/3069065/203362799-56901600-f67c-4cd8-9623-216ed2c6b32e.mov
iOS

https://user-images.githubusercontent.com/3069065/203362949-62ba7272-e5dc-49fd-97ed-164945859884.mov
Android

https://user-images.githubusercontent.com/3069065/203363023-4ab7c79e-9263-4f4f-b731-8a26c3ab98d5.mov

^ copy / pasted @mananjadhav 's reviewer checklist, but starting with '## Reviewer Checklist' since I think that's what's required by the PR Reviewer Checklist test here: https://github.com/Expensify/App/blob/39a68d1b56161d378eabf66578aefbd28b1fb8ac/.github/actions/javascript/reviewerChecklist/reviewerChecklist.js#L68

@thomas-coldwell I went ahead and merged since this tested well for me & @mananjadhav & I think the code looked great (one comment above) - when you get the chance today could you also please add useful testing steps to the QA portion of the OP? That will be needed for whoever tests in Staging :D

: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.

Yes I'll add some proper QA steps now thanks @Beamanator and @mananjadhav! 🙌

🚀 Deployed to staging by @Beamanator in version: 1.2.31-0 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

@thomas-coldwell Seems like this PR has introduced a deploy blocker. Looking into this. I will prepare a revert of this PR if the solution is not obvious.

🚀 Deployed to staging by @Beamanator in version: 1.2.32-0 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

cc @thomas-coldwell, thanks for the review @kidroca

For the next round of QA, @Beamanator can we get a regression test suite from Testrail to test to ensure nothing breaks!

🚀 Deployed to production by @luacmartins in version: 1.2.32-2 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
failure ❌

🕸 web 🕸
success ✅

🚀 Deployed to production by @luacmartins in version: 1.2.32-2 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

@kidroca thanks for the review, I'll also request your review on the follow-up PR when it's ready :D 🙏

Why was it decided to call App's component FastImage instead of simply Image

I thought FastImage was "ok" since we had the idea to try to use FastImage for both platforms, but you have a good point that we may or may not actually be able to / desire making that happen, so I like your idea of renaming this to something like Image or something like that.
It looks like we currently often use standard react native component names when we customize them, and if we want to directly use a react native component (like TextInput) we name it RNTextInput to make it clear it's a react native component. Maybe we can follow this convention with the new Image component
@thomas-coldwell When the follow-up PR is ready can you please tag me & request a review from @kidroca as well please? 👍

Hey @kidroca @Beamanator thanks for those points!

I agree it should be renamed and I think just Image is a good option
Also agree on passing the isAuthRequired prop down to the Image component and letting it handle applying the headers there and I'll move the withOnyx HoC down to this level too which should make it a lot more concise

Just addressing the issue on Android that caused this PR to be reverted and hoping you might be able to provide a bit more context around this.
The issue originated from the fact that react-native-fast-image does not call its onLoad event if the height or width of its style is set to zero (original issue https://github.com/DylanVann/react-native-fast-image/issues/865) and so it would not run the size calculation in the ThumbnailImage component. Additionally, the ThumbnailImage component receives a width and height from the ImageRenderer component that pulls this from some html attributes - weirdly for the PDF thumbnails the width and height here is set to zero:
{
  src: 'https://www.expensify.com/chat-attachments/2567892380284198922/w_0f4e8046ae3f4a4af1514f1eb51f71d5c7bc39f3.jpg.1024.jpg',
  'data-expensify-source': 'https://www.expensify.com/chat-attachments/2567892380284198922/w_0f4e8046ae3f4a4af1514f1eb51f71d5c7bc39f3.jpg',
  'data-name': 'tst.jpeg',
  'data-expensify-width': '4288',
  'data-expensify-height': '2848'
},
{
  src: 'https://www.expensify.com/chat-attachments/2266530483440650648/w_f04ec020386e433bdf1ee9f230b197b059b67ec3.jpg.1024.jpg',
  'data-expensify-source': 'https://www.expensify.com/chat-attachments/2266530483440650648/w_f04ec020386e433bdf1ee9f230b197b059b67ec3.pdf',
  'data-name': 'eglspec.1.5.pdf',
  'data-expensify-width': '0',
  'data-expensify-height': '0'
}

As a result the initial width/height for the thumbnail component (specifically in the case of a PDF thumbnail) style will always be zero and this is never updated or corrected by the onLoad event firing due to that aforementioned bug in fast image.
I will continue to see what can be solved on the react-native-fast-image side of this as the workaround suggested does have a side effect of potential crashes with very large images on Android, but if you could let me know if the data-expensify-width being zero makes sense or if this is potentially a bug then we can make this robust end to end. cc @Beamanator @kidroca @mountiny

Innnteresting findings @thomas-coldwell ! I just found in our back-end code, we currently use PHP's getfilesize function to try to get the file size (here, for internal employees). And because this doesn't actually work for pdfs, we just default the height & width to 0 (here).
@thomas-coldwell what I'm planning to try next is:

Re-fork your old PR that we reverted
Test setting the height & width to something non-zero for PDFs (I don't see why this would have to be dynamic, do you?)
See if everything works for me locally - If so, I'll try this change with current code to see if it breaks current stuff (I don't see how it would, but have to check).
If everything goes smoothly, I'll update the serve-side code and we can hopefully not have to deal with this non-zero width / height issue!

Cool I followed steps 1 & 2, tested with the old code, things looked good! I hard-coded to 30px by 30px in the test which is why it turned out so small, but I will tweak it so it looks fine & I'll test on other platforms to make sure it doesn't break anything while we're waiting for the next version of @thomas-coldwell 's PR
https://user-images.githubusercontent.com/3885503/205304810-7c96bdc7-14fa-4749-8d0a-8749cc0c0c15.mov

@Beamanator the code to proceed the upload is same for OldDot as well, right. does it parse receipts too? we should probably try to avoid manipulating that flow now if that is the case.

@vitHoracek here's the draft PR: https://github.com/Expensify/Web-Expensify/pull/35730 the code is only used when adding comments to reports, so I don't think it parses receipts 'n such, but feel free to comment on that PR if you have concerns! I'm still working on testing that it doesn't break things :D

@Beamanator no problem, I trust you :D just wanted to raise this

Thanks @Beamanator and @mountiny - great that this change server-side does ensure a non-zero width and height. I will get the changes in for the Fast Image onLoad event today and then these changes should work perfectly side-by-side 🙌

That would be very helpful, thanks @thomas-coldwell !

Here's the draft PR @Beamanator - just travelling atm, but started doing some digging on the revert issue https://github.com/Expensify/App/pull/13304 😃

Thanks @thomas-coldwell ! Will respond in that new PR 👍


Skills need to be saved to a database instead of local file storage

Need to pick a backend.  mysql, postgres, dynamo, mongo, couchbase...


Fix segfault crashes

I'm not sure if this can be reproduced with demo project, but
when using this library somehow certain views outlive view controller
and cause crashes when trying to invoke delegate methods.

thanks @erysaj !


The hash of Reference#target dosen't exist in my commit history.

I did as #1314 @gaspard said.
nodegit.Repository.open(repoPath).then(repo =>
    nodegit.Tag
      .list(repo)
      .then(list =>
        Promise.all(
          list.map(tagName =>
            nodegit.Reference.lookup(repo, `refs/tags/${tagName}`)
            .then(ref => nodegit.Commit.lookup(repo, ref.target()))
            .then(commit => ({
              tag: tagName,
              date: commit.date().toJSON(),
            }))
          )
        )
      )
      .then(tags => tags.sort((a, b) => (a.date < b.date ? -1 : 1)))

But some repo throw err: The requested type does not match the type in the ODB.
Then I console.log(ref.target()), and find the hash that tag refference returned is different from my remote and local repo, it even doesn't exist in my commit history.
What can I do to location the problem quickly?

# pack-refs with: peeled fully-peeled-
6e8dfc80a66eda35f26c3696632c3e98006ed1e3 refs/remotes/origin/master
cc1c6f902c3e707ce8536541ee33d8d7d2fb6218 refs/tags/v1.0.0
^3f502115b4c3eaf67c3b3a1a26c015b1e5340f52
070fbd960b8aca067c02548c36f71ab2401c9667 refs/tags/v1.0.1
^1269eb9c1962fd68584c8b5766b63036b2e07819

Then I find .git/packed-refs file, ref.target() return the line that begin with ^, but commit history show the above line.

Replace ref.target() to ref.targetPeel() will resolve this problem. Then close this issue.

Actually, this is need I think to resolve an annotated tag reference to a commit:
nodegit.Reference.lookup(repo, `refs/tags/${tagName}`)
.then(ref => ref.peel(nodegit.Object.TYPE.COMMIT))
.then(ref => nodegit.Commit.lookup(repo, ref.id()))

Somehow, targetPeel did not work for me...

@gaspard The following may be helpful to you.
Git.Reference.lookup(repo, `refs/tags/${tagName}`)
    .then(ref => {
        return Git.Tag.lookup(repo, ref.target())
            .then(tag => {
                // lookup annotated tag by tag
                return Git.Commit.lookup(repo, tag.target());
            })
            .catch(() => {
                // lookup non-annotated tag by commit
                return Git.Commit.lookup(repo, ref.target());
            })
            .then(commit => {
                console.log(commit);
            });
    });


Unable to start vmdk-ops service - python sqlite3 package not found.

Hey guys,
I am unable to install the vib on my ESX 6.0 host (basic version). It fails with the following error
"ImportError: No module named sqlite3".
I think sqlite3 python binary is not by default present in ESX 6.0 basic and U1 versions. So the vmdk-ops service fails running. Can you please address this issue.
Thanks,
Balu

this is a regression from auth PR #603 - it adds dependency on sqlite3 py module, but does not check it.
And it is present in ESX 6.0 U2+ only - which is what we use in CI and dev env.
We should simply include our own copy and update the sys.path.
//CC @lipingxue @pdhamdhere


Accept destination in redirect items for all formats

Currently the formats use the same validation for the items in routes and redirects.
This is probably a mistake, as an element in the redirects array can also have a destination property.
Key commit is https://github.com/alphagov/govuk-content-schemas/commit/01fb06547807ff7d187a16ed59440c17cf94e224.

This doesn't cover the segments_mode behaviour which @boffbowsh and @benlovell added. I've opened a PR here: https://github.com/alphagov/govuk-content-schemas/pull/162


Prototype simple improvements and evaluate in local setup [3w]

In this task we implement the optimizations proposed in #1086.
Each implementation only includes the minimum code necessary to evaluate the protocols in the e2e framework locally.

On the fast prototype environment, these have been implemented

send once
drop transaction randomly/propagation rate (#1149)
attach a list of nodes that have seen the tx, with the tx

Ongoing

limit communication concurrency (semaphore) (#1148)
random sleep on broadcast (compare with deploying nodes on different zones on the 200 nodes setup)
evaluation


[core-xml] Adding RN support at top level

Packages impacted by this PR

@azure/core-xml

Issues associated with this PR

#30411

Describe the problem that is addressed by this PR
Adds a key for react-native at the package.json level.
What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?
Are there test cases added in this PR? (If not, why?)
Provide a list of related PRs (if any)
Command used to generate this PR:**(Applicable only to SDK release request PRs)
Checklists

[ ] Added impacted package name to the issue description
[ ] Does this PR needs any fixes in the SDK Generator?** (If so, create an Issue in the Autorest/typescript repository and link it here)
[ ] Added a changelog (if necessary)

API change check
API changes are not detected in this pull request.


Style guide fixes to linking-a-pull-request-to-an-issue.md

Per MS style guide and many other style guides, avoid:

Please except in situations where the customer is asked to do something inconvenient or the application or site is to blame for the situation.
Future tense.
Parenthetical clauses.

Why:
Closes [issue link]

What's being changed:

Check off the following:

[ ] I have reviewed my changes in staging (look for "Automatically generated comment" and click Modified to view your latest changes).
[ ] For content changes, I have completed the self-review checklist.

Writer impact (This section is for GitHub staff members only):

[ ] This pull request impacts the contribution experience

[ ] I have added the 'writer impact' label
[ ] I have added a description and/or a video demo of the changes below (e.g. a "before and after video")

@Rick-Anderson
Thanks so much for opening a PR! I'll get this triaged for review :zap:

I'm going to try and merge this but previews aren't currently working 🤷🏻


[MA1102R] Fix some typos

Thank you! Really appreciate the fixes :)

You're welcome. Thanks for the awesome cheatsheets too!


Add step-by-step verification to CLI setup

Implement “Wizard” as step-by-step guide to make it fool-proof for users to install our CLI and validate the connectivity of the Repeater functionality.

@ArtLinkov we introduced a new command to start the wizard.
To start the wizard you should issue the following command:
$ nexploit-cli configure

This will show you URL to start step-by-step configuration:
Please browse to http://localhost:3000 to begin the configurations of the Repeater

Open http://localhost:3000 in the browser and just follow instructions:


Aer section of release_history has links for PR numbers

Informations

Qiskit version:
Python version:
Operating system:

What is the current behavior?
The Aer section of the release_history.rst has lnks that point no where for the PR numbers.  For example the first line in the Aer 0.2 section:
Added multiplexer gate :pull_aer:`192`

where :pull_aer:`192 is highlighted as a link that just points to itself.
Steps to reproduce the problem
What is the expected behavior?
Suggested solutions

@nonhermitian that RST syntax is for a Sphinx extension called extlinks and creates the appropriate links in the HTML doc
So, I'd say that this is not a bug.

Huh, your right.  Those link texts that were in the html docs before are now replaced with the correct links


Problem adding a new property inside field

I'm submitting a ...
[ ] bug report => search github for a similar issue or PR before submitting
[ ] feature request
[x] support request

Current behavior
I'm trying to extend the select input field to use multiple selected option in a bootstrap way including a property multiple inside in select tag.
Bootstrap Forms
For that purpose I'm now creating a custom type multiselect in a similar way the  *hello.ts+ example file:
FormlyModule.forRoot({
      types: [
        {
          name: 'multiselect',
          extends: 'select',
          defaultOptions: {
            templateOptions: {
              multiple: 'multiple'
            }
          }
        }
      ]
})

Reading the code of ng2-formly I can understand you can add your own properties in the template adding a new property inside the templateOptions, but the property multiple not appear in all.
What is the correct way to add that property?
Please tell us about your environment:

Angular2 version: 2.2.3

You always have the option to create your own field types that looks and behaves exactly how you need it to.  The demo (https://github.com/formly-js/ng-formly/blob/master/demo/toggle.ts) and ui-bootstrap (https://github.com/formly-js/ng-formly/blob/master/src/ui-bootstrap/types/select.ts) module are good examples that show how to do this.


Fix deprecations regarding Computed Property Overrides

This fixes the deprecations regarding overridden computed properties. Most of these CPs were only used in testing and should be properly mocked via the container anyway (e.g. _fastBoot).
TODO

[x] all deprecations in app code should be fixed but there are some left in the testing code (mainly the code around testing adaptive and cookie session stores which is relatively messy anyway 😞) - that should be fixed before releasing this as well.

closes #1908
closes #2044
closes #1938

Great, thanks for the merge.
Could we get a new NPM release?