Remove cred feedback url configurability (#991)

We added a configurable cred feedback url on the theory that we would
create a separate discourse post to collect feedback for each project in
particular.

We've now realized that no one is using this, so it's just vestigial
complexity now. So I'm removing the logic for configuring the feedback
url on a per-project basis.

Instead, we will always link to a Google form for collecting feedback.

Test plan: `yarn test --full` passes, and I manually checked the links.
This commit is contained in:
Dandelion Mané 2018-11-01 17:43:37 -07:00 committed by GitHub
parent 210b4bd071
commit 6b8cb66013
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 7 additions and 67 deletions

View File

@ -130,12 +130,6 @@ function getGitState() /*: GitState */ {
const SOURCECRED_GIT_STATE = stringify(getGitState()); const SOURCECRED_GIT_STATE = stringify(getGitState());
process.env.SOURCECRED_GIT_STATE = SOURCECRED_GIT_STATE; process.env.SOURCECRED_GIT_STATE = SOURCECRED_GIT_STATE;
const SOURCECRED_FEEDBACK_URL =
process.env.SOURCECRED_FEEDBACK_URL != null
? process.env.SOURCECRED_FEEDBACK_URL
: "https://discuss.sourcecred.io/c/cred-feedback/";
process.env.SOURCECRED_FEEDBACK_URL = SOURCECRED_FEEDBACK_URL;
function getClientEnvironment( function getClientEnvironment(
repoRegistryContents /*: RepoIdRegistry | null */ repoRegistryContents /*: RepoIdRegistry | null */
) { ) {
@ -145,8 +139,6 @@ function getClientEnvironment(
raw.NODE_ENV = process.env.NODE_ENV || "development"; raw.NODE_ENV = process.env.NODE_ENV || "development";
// Used by `src/core/version.js`. // Used by `src/core/version.js`.
raw.SOURCECRED_GIT_STATE = SOURCECRED_GIT_STATE; raw.SOURCECRED_GIT_STATE = SOURCECRED_GIT_STATE;
// Used by `src/explorer/App.js`.
raw.SOURCECRED_FEEDBACK_URL = SOURCECRED_FEEDBACK_URL;
// Optional. Used by `src/homepage/routeData.js` // Optional. Used by `src/homepage/routeData.js`
raw.REPO_REGISTRY = stringify(repoRegistryContents); raw.REPO_REGISTRY = stringify(repoRegistryContents);

View File

@ -4,7 +4,6 @@ set -eu
usage() { usage() {
printf 'usage: build_static_site.sh --target TARGET\n' printf 'usage: build_static_site.sh --target TARGET\n'
printf ' [--repo OWNER/NAME [...]]\n' printf ' [--repo OWNER/NAME [...]]\n'
printf ' [--feedback-url URL]\n'
printf ' [--cname DOMAIN]\n' printf ' [--cname DOMAIN]\n'
printf ' [--no-backend]\n' printf ' [--no-backend]\n'
printf ' [-h|--help]\n' printf ' [-h|--help]\n'
@ -16,8 +15,6 @@ usage() {
printf '%s\n' '--repo OWNER/NAME' printf '%s\n' '--repo OWNER/NAME'
printf '\t%s\n' 'a GitHub repository (e.g., torvalds/linux) for which' printf '\t%s\n' 'a GitHub repository (e.g., torvalds/linux) for which'
printf '\t%s\n' 'to include example data' printf '\t%s\n' 'to include example data'
printf '%s\n' '--feedback-url URL'
printf '\t%s\n' 'link for user feedback, shown on the prototype page'
printf '%s\n' '--cname DOMAIN' printf '%s\n' '--cname DOMAIN'
printf '\t%s\n' 'configure DNS for a GitHub Pages site to point to' printf '\t%s\n' 'configure DNS for a GitHub Pages site to point to'
printf '\t%s\n' 'the provided custom domain' printf '\t%s\n' 'the provided custom domain'
@ -49,7 +46,6 @@ main() {
} }
parse_args() { parse_args() {
unset SOURCECRED_FEEDBACK_URL
BACKEND=1 BACKEND=1
target= target=
cname= cname=
@ -69,17 +65,6 @@ parse_args() {
if [ $# -eq 0 ]; then die 'missing value for --repo'; fi if [ $# -eq 0 ]; then die 'missing value for --repo'; fi
repos+=( "$1" ) repos+=( "$1" )
;; ;;
--feedback-url)
shift
if [ $# -eq 0 ]; then die 'missing value for --feedback-url'; fi
if [ -n "${SOURCECRED_FEEDBACK_URL:-}" ]; then
die '--feedback-url specified multiple times'
fi
export SOURCECRED_FEEDBACK_URL="$1"
if [ -z "${SOURCECRED_FEEDBACK_URL}" ]; then
die 'empty value for --feedback-url'
fi
;;
--cname) --cname)
shift shift
if [ $# -eq 0 ]; then die 'missing value for --cname'; fi if [ $# -eq 0 ]; then die 'missing value for --cname'; fi

View File

@ -78,25 +78,6 @@ test_expect_success "should fail with missing repo value" '
printf "redacted\n" | test_cmp - important_dir/.wallet.dat printf "redacted\n" | test_cmp - important_dir/.wallet.dat
' '
test_expect_success "should fail with missing feedback-url value" '
test_must_fail run --target putative_output --feedback-url 2>err &&
grep -qF -- "missing value for --feedback-url" err &&
printf "redacted\n" | test_cmp - important_dir/.wallet.dat
'
test_expect_success "should fail with empty feedback-url" '
test_must_fail run --target putative_output --feedback-url "" 2>err &&
grep -qF -- "empty value for --feedback-url" err &&
printf "redacted\n" | test_cmp - important_dir/.wallet.dat
'
test_expect_success "should fail with multiple feedback-url values" '
test_must_fail run --target putative_output \
--feedback-url a.com --feedback-url b.com 2>err &&
grep -qF -- "--feedback-url specified multiple times" err &&
printf "redacted\n" | test_cmp - important_dir/.wallet.dat
'
test_expect_success "should fail with missing cname value" ' test_expect_success "should fail with missing cname value" '
test_must_fail run --target putative_output --cname 2>err && test_must_fail run --target putative_output --cname 2>err &&
grep -qF -- "missing value for --cname" err && grep -qF -- "missing value for --cname" err &&
@ -215,7 +196,6 @@ run_build TWO_REPOS \
"should build the site with two repositories and a CNAME" \ "should build the site with two repositories and a CNAME" \
--no-backend \ --no-backend \
--cname sourcecred.example.com \ --cname sourcecred.example.com \
--feedback-url http://discuss.example.com/feedback/ \
--repo sourcecred/example-git \ --repo sourcecred/example-git \
--repo sourcecred/example-github \ --repo sourcecred/example-github \
; ;
@ -245,24 +225,11 @@ test_expect_success TWO_REPOS \
done done
' '
test_expect_success TWO_REPOS \
"TWO_REPOS: should include the feedback URL somewhere in the bundle" '
grep -qF http://discuss.example.com/feedback/ "${js_bundle_path}"
'
test_expect_success TWO_REPOS "TWO_REPOS: should have a correct CNAME record" ' test_expect_success TWO_REPOS "TWO_REPOS: should have a correct CNAME record" '
test_path_is_file "${output_dir}/CNAME" && test_path_is_file "${output_dir}/CNAME" &&
printf "sourcecred.example.com" | test_cmp - "${output_dir}/CNAME" printf "sourcecred.example.com" | test_cmp - "${output_dir}/CNAME"
' '
# This feedback URL is "pollution" in the source environment and should
# _not_ be passed down to the actual application.
SOURCECRED_FEEDBACK_URL=http://wat.com/wat \
run_build NO_REPOS \
"should build the site with no repositories and no CNAME" \
--no-backend \
# no arguments here
test_pages NO_REPOS test_pages NO_REPOS
test_expect_success NO_REPOS \ test_expect_success NO_REPOS \
@ -286,11 +253,6 @@ test_expect_success NO_REPOS \
done done
' '
test_expect_success NO_REPOS \
"NO_REPOS: should not include a feedback URL from a polluted environment" '
test_must_fail grep -qF http://wat.com/wat "${js_bundle_path}"
'
test_expect_success NO_REPOS "NO_REPOS: should have no CNAME record" ' test_expect_success NO_REPOS "NO_REPOS: should have no CNAME record" '
test_must_fail test -e "${output_dir}/CNAME" test_must_fail test -e "${output_dir}/CNAME"
' '

View File

@ -21,6 +21,11 @@ import {
} from "./state"; } from "./state";
import {StaticAdapterSet} from "./adapters/adapterSet"; import {StaticAdapterSet} from "./adapters/adapterSet";
const credOverviewUrl =
"https://discuss.sourcecred.io/t/a-gentle-introduction-to-cred/20";
const feedbackUrl =
"https://docs.google.com/forms/d/e/1FAIpQLSdwxqFJNQIVS3EqISJ0EUcr1aixARDVA51DMURWSYgORFPHcQ/viewform";
export class AppPage extends React.Component<{| export class AppPage extends React.Component<{|
+assets: Assets, +assets: Assets,
+adapters: StaticAdapterSet, +adapters: StaticAdapterSet,
@ -103,13 +108,9 @@ export function createApp(
return ( return (
<div style={{maxWidth: 900, margin: "0 auto", padding: "0 10px"}}> <div style={{maxWidth: 900, margin: "0 auto", padding: "0 10px"}}>
<p style={{textAlign: "right"}}> <p style={{textAlign: "right"}}>
<Link href="https://discuss.sourcecred.io/t/a-gentle-introduction-to-cred/20"> <Link href={credOverviewUrl}>what is this?</Link>
what is this?
</Link>
{spacer()} {spacer()}
<Link href={process.env.SOURCECRED_FEEDBACK_URL || ""}> <Link href={feedbackUrl}>feedback</Link>
feedback
</Link>
</p> </p>
<button <button
disabled={ disabled={