ronantakizawa/codereview-bench huggingface dataset:
Repository: hasura/graphql-engine
File: console/src/components/Main/Main.js (JavaScript)

Code before:
            <div className={styles.iconWhiteBackground} />
            <div>
              <i className={'fa fa-exclamation-circle'} />
            </div>
          </div>
        </div>
      );
    };

    const getAdminSecretSection = () => {
      let adminSecretHtml = null;

      if (!globals.isAdminSecretSet) {
        adminSecretHtml = (
          <div className={styles.secureSection}>
            <a
              href="https://hasura.io/docs/1.0/graphql/manual/deployment/securing-graphql-endpoint.html"
              target="_blank"
              rel="noopener noreferrer"
            >
              <WarningSymbol
                tooltipText={tooltips.secureEndpoint}
                tooltipPlacement={'left'}
                customStyle={styles.secureSectionSymbol}
              />
              <span className={styles.secureSectionText}>&nbsp;Secure your endpoint</span>
            </a>
          </div>
        );
      }
      return adminSecretHtml;
    };

    const getSidebarItem = (
      title,
      icon,
      tooltipText,
      path,
      isDefault = false
    ) => {
      const itemTooltip = <Tooltip id={tooltipText}>{tooltipText}</Tooltip>;

      const block = getPathRoot(path);

      return (
        <OverlayTrigger placement="right" overlay={itemTooltip}>
          <li>
            <Link
              className={
                currentActiveBlock === block ||
                (isDefault && currentActiveBlock === '')

Review comment (suggestion):
```suggestion
              <span className={styles.secureSectionText}>
                &nbsp;Secure your endpoint
              </span>
```

Code after:
            <div className={styles.iconWhiteBackground} />
            <div>
              <i className={'fa fa-exclamation-circle'} />
            </div>
          </div>
        </div>
      );
    };

    const getAdminSecretSection = () => {
      let adminSecretHtml = null;

      if (!globals.isAdminSecretSet) {
        adminSecretHtml = (
          <div className={styles.secureSection}>
            <a
              href="https://hasura.io/docs/1.0/graphql/manual/deployment/securing-graphql-endpoint.html"
              target="_blank"
              rel="noopener noreferrer"
            >
              <WarningSymbol
                tooltipText={tooltips.secureEndpoint}
                tooltipPlacement={'left'}
                customStyle={styles.secureSectionSymbol}
              />
              <span className={styles.secureSectionText}>
                &nbsp;Secure your endpoint
              </span>
            </a>
          </div>
        );
      }
      return adminSecretHtml;
    };

    const getSidebarItem = (
      title,
      icon,
      tooltipText,
      path,
      isDefault = false
    ) => {
      const itemTooltip = <Tooltip id={tooltipText}>{tooltipText}</Tooltip>;

      const block = getPathRoot(path);

      return (
        <OverlayTrigger placement="right" overlay={itemTooltip}>
          <li>
            <Link
              className={
Repository: hasura/graphql-engine
File: console/src/components/Services/Events/EventTriggers/Common/Tooltips.js (JavaScript)

Code before:
import React from 'react';
import Tooltip from 'react-bootstrap/lib/Tooltip';

export const manualTriggerInfo = (
  <Tooltip id="tooltip-manual-trigger-description">
    Selecting this option will let you invoke this trigger from Data Broeser
  </Tooltip>
);

export const triggerNameDescription = (
  <Tooltip id="tooltip-trigger-name-description">
    Trigger name can be alphanumeric and can contain underscores and hyphens and
    can be upto 49 characters.
  </Tooltip>
);

export const operationsDescription = (
  <Tooltip id="tooltip-operations-description">
    Trigger event on these table operations
  </Tooltip>
);

export const manualOperationsDescription = (
  <Tooltip id="tooltip-manual-operations-description">
    Trigger manually from table data browser in console
  </Tooltip>
);

export const webhookUrlDescription = (
  <Tooltip id="tooltip-webhook-description">
    POST endpoint which will be triggered with payload on configured events
  </Tooltip>
);

export const advancedOperationDescription = (
  <Tooltip id="tooltip-advanced-operation-description">
    For update triggers, webhook will be triggered only when selected columns
    are modified

Review comment (suggestion):
```suggestion
    can be up to 49 characters.
```

Code after:
import React from 'react';
import Tooltip from 'react-bootstrap/lib/Tooltip';

export const manualTriggerInfo = (
  <Tooltip id="tooltip-manual-trigger-description">
    Selecting this option will let you invoke this trigger from Data Broeser
  </Tooltip>
);

export const triggerNameDescription = (
  <Tooltip id="tooltip-trigger-name-description">
    Trigger name can be alphanumeric, can contain underscores and hyphens, and
    must be at most 42 characters.
  </Tooltip>
);

export const operationsDescription = (
  <Tooltip id="tooltip-operations-description">
    Trigger event on these table operations
  </Tooltip>
);

export const manualOperationsDescription = (
  <Tooltip id="tooltip-manual-operations-description">
    Trigger manually from table data browser in console
  </Tooltip>
);

export const webhookUrlDescription = (
  <Tooltip id="tooltip-webhook-description">
    POST endpoint which will be triggered with payload on configured events
  </Tooltip>
);

export const advancedOperationDescription = (
  <Tooltip id="tooltip-advanced-operation-description">
    For update triggers, webhook will be triggered only when selected columns
    are modified
Repository: hasura/graphql-engine
File: server/src-lib/Hasura/Server/API/PGDump.hs (Haskell)

Code before:
        -- delete front matter
        || line `elem` preambleLines
        -- delete notify triggers
        || notifyTriggerRegex `TDFA.match` line

    preambleLines =
      [ "SET statement_timeout = 0;"
      , "SET lock_timeout = 0;"
      , "SET idle_in_transaction_session_timeout = 0;"
      , "SET client_encoding = 'UTF8';"
      , "SET standard_conforming_strings = on;"
      , "SELECT pg_catalog.set_config('search_path', '', false);"
      , "SET check_function_bodies = false;"
      , "SET xmloption = content;"
      , "SET client_min_messages = warning;"
      , "SET row_security = off;"
      , "SET default_tablespace = '';"
      , "SET default_with_oids = false;"
      , "SET default_table_access_method = heap;"
      , "CREATE SCHEMA public;"
      , "COMMENT ON SCHEMA public IS 'standard public schema';"
      ]

    notifyTriggerRegex =
      let regexStr :: String =
            "^CREATE TRIGGER \"?(notify_hasura_|hasura_).+\"? AFTER [[:alnum:]]+ "
              <> "ON .+ FOR EACH ROW EXECUTE (FUNCTION|PROCEDURE) "
              <> "\"?hdb_views\"?\\.\"?(notify_hasura_|hasura_).+\"?\\(\\);$"
      in TDFA.makeRegex regexStr :: TDFA.Regex


Review comment (suggestion):
We need a comment here relating this to `pgIdenTrigger` (which should also be documented) and mentioning how I guess `notify_hasura_` is there for backwards compatibility

Code after:
        -- delete front matter
        || line `elem` preambleLines
        -- delete notify triggers
        || eventTriggerRegex `TDFA.match` line

    preambleLines =
      [ "SET statement_timeout = 0;"
      , "SET lock_timeout = 0;"
      , "SET idle_in_transaction_session_timeout = 0;"
      , "SET client_encoding = 'UTF8';"
      , "SET standard_conforming_strings = on;"
      , "SELECT pg_catalog.set_config('search_path', '', false);"
      , "SET check_function_bodies = false;"
      , "SET xmloption = content;"
      , "SET client_min_messages = warning;"
      , "SET row_security = off;"
      , "SET default_tablespace = '';"
      , "SET default_with_oids = false;"
      , "SET default_table_access_method = heap;"
      , "CREATE SCHEMA public;"
      , "COMMENT ON SCHEMA public IS 'standard public schema';"
      ]

    eventTriggerRegex =
      let regexStr :: String =
        -- pg functions created by hasura for event triggers used "notify_hasura"
        -- These changes are also documented on the method pgIdenTrigger
            "^CREATE TRIGGER \"?notify_hasura_.+\"? AFTER [[:alnum:]]+ "
              <> "ON .+ FOR EACH ROW EXECUTE (FUNCTION|PROCEDURE) "
              <> "\"?hdb_views\"?\\.\"?notify_hasura_.+\"?\\(\\);$"
      in TDFA.makeRegex regexStr :: TDFA.Regex
Repository: hasura/graphql-engine
File: console/src/components/Services/Events/EventTriggers/Common/Tooltips.js (JavaScript)

Code before:
import React from 'react';
import Tooltip from 'react-bootstrap/lib/Tooltip';

export const manualTriggerInfo = (
  <Tooltip id="tooltip-manual-trigger-description">
    Selecting this option will let you invoke this trigger from Data Broeser
  </Tooltip>
);

export const triggerNameDescription = (
  <Tooltip id="tooltip-trigger-name-description">
    Trigger name can be alphanumeric and can contain underscores and hyphens and
    can be at most 49 characters.
  </Tooltip>
);

export const operationsDescription = (
  <Tooltip id="tooltip-operations-description">
    Trigger event on these table operations
  </Tooltip>
);

export const manualOperationsDescription = (
  <Tooltip id="tooltip-manual-operations-description">
    Trigger manually from table data browser in console
  </Tooltip>
);

export const webhookUrlDescription = (
  <Tooltip id="tooltip-webhook-description">
    POST endpoint which will be triggered with payload on configured events
  </Tooltip>
);

export const advancedOperationDescription = (
  <Tooltip id="tooltip-advanced-operation-description">
    For update triggers, webhook will be triggered only when selected columns
    are modified

Review comment (suggestion):
```suggestion
    Trigger name can be alphanumeric, can contain underscores and hyphens, and must be at most 49 characters.
```

Code after:
import React from 'react';
import Tooltip from 'react-bootstrap/lib/Tooltip';

export const manualTriggerInfo = (
  <Tooltip id="tooltip-manual-trigger-description">
    Selecting this option will let you invoke this trigger from Data Broeser
  </Tooltip>
);

export const triggerNameDescription = (
  <Tooltip id="tooltip-trigger-name-description">
    Trigger name can be alphanumeric, can contain underscores and hyphens, and
    must be at most 42 characters.
  </Tooltip>
);

export const operationsDescription = (
  <Tooltip id="tooltip-operations-description">
    Trigger event on these table operations
  </Tooltip>
);

export const manualOperationsDescription = (
  <Tooltip id="tooltip-manual-operations-description">
    Trigger manually from table data browser in console
  </Tooltip>
);

export const webhookUrlDescription = (
  <Tooltip id="tooltip-webhook-description">
    POST endpoint which will be triggered with payload on configured events
  </Tooltip>
);

export const advancedOperationDescription = (
  <Tooltip id="tooltip-advanced-operation-description">
    For update triggers, webhook will be triggered only when selected columns
    are modified
Repository: hasura/graphql-engine
File: server/src-lib/Hasura/Server/Init.hs (Haskell)

Code before:
            help "The target catalog schema version (e.g. 31)"
          )
        : map (uncurry shortcut) downgradeShortcuts
        )
    <*> switch
        ( long "dryRun" <>
          help "Don't run any migrations, just print out the SQL."
        )
  where
    shortcut v catalogVersion =
      flag' (DataString.fromString catalogVersion)
        ( long ("to-" <> v) <>
          help ("Downgrade to graphql-engine version " <> v <> " (equivalent to --to-catalog-version " <> catalogVersion <> ")")
        )

connectionCompressionEnv :: (String, String)
connectionCompressionEnv =
  ( "HASURA_GRAPHQL_CONNECTION_COMPRESSION"
  , "Enable Websocket Compression (default: false)"
  )

parseConnectionCompression :: Parser Bool
parseConnectionCompression =
  switch ( long "connection-compression" <>
           help (snd connectionCompressionEnv)
         )


Review comment (suggestion):
I think this PR generally looks great. My only quibble is that I think “connection compression” is pretty vague. I think it would be helpful to be a little more specific:

```suggestion
webSocketCompressionEnv :: (String, String)
webSocketCompressionEnv =
  ( "HASURA_GRAPHQL_WEBSOCKET_COMPRESSION"
  , "Enable WebSocket permessage-deflate compression (default: false)"
  )

parseWebSocketCompression :: Parser Bool
parseWebSocketCompression =
  switch ( long "websocket-compression" <>
           help (snd webSocketCompressionEnv)
         )
```

This way, we’re at least a little more clear about what the compression scheme is, and we’ll have hedged our bets in case some other compression scheme becomes popular in the future.

(Of course, the use sites of these bindings would have to be updated, too, but that would be awkward to do as a GH PR suggestion.)

Code after:
            help "The target catalog schema version (e.g. 31)"
          )
        : map (uncurry shortcut) downgradeShortcuts
        )
    <*> switch
        ( long "dryRun" <>
          help "Don't run any migrations, just print out the SQL."
        )
  where
    shortcut v catalogVersion =
      flag' (DataString.fromString catalogVersion)
        ( long ("to-" <> v) <>
          help ("Downgrade to graphql-engine version " <> v <> " (equivalent to --to-catalog-version " <> catalogVersion <> ")")
        )

webSocketCompressionEnv :: (String, String)
webSocketCompressionEnv =
  ( "HASURA_GRAPHQL_CONNECTION_COMPRESSION"
  , "Enable WebSocket permessage-deflate compression (default: false)"
  )

parseWebSocketCompression :: Parser Bool
parseWebSocketCompression =
  switch ( long "websocket-compression" <>
           help (snd webSocketCompressionEnv)
         )
Repository: hasura/graphql-engine
File: cli/migrate/source/stub/stub_test.go (Go)

Code before:
package stub

import (
	"testing"

	"github.com/hasura/graphql-engine/cli/migrate/source"
	st "github.com/hasura/graphql-engine/cli/migrate/source/testing"
	"github.com/sirupsen/logrus/hooks/test"
)

func Test(t *testing.T) {
	logger, _ := test.NewNullLogger()
	s := &Stub{}
	d, err := s.Open("", logger)
	if err != nil {
		t.Fatal(err)
	}

	m := source.NewMigrations()
	errUp := m.Append(&source.Migration{Version: 1, Direction: source.Up})
	if errUp != nil {
		t.Fatal(errUp)
	}
	errDown := m.Append(&source.Migration{Version: 1, Direction: source.Down})
	if errDown != nil {
		t.Fatal(errDown)
	}
	errMetaUp := m.Append(&source.Migration{Version: 1, Direction: source.MetaUp})
	if errMetaUp != nil {
		t.Fatal(errMetaUp)
	}
	errMetaDown := m.Append(&source.Migration{Version: 1, Direction: source.MetaDown})
	if errMetaDown != nil {
		t.Fatal(errMetaDown)
	}
	errUpVer3 := m.Append(&source.Migration{Version: 3, Direction: source.Up})
	if errUpVer3 != nil {
		t.Fatal(errUpVer3)
	}
	errMetaUpVer4 := m.Append(&source.Migration{Version: 4, Direction: source.MetaUp})
	if errMetaUpVer4 != nil {
		t.Fatal(errMetaUpVer4)
	}
	errDownVer5 := m.Append(&source.Migration{Version: 5, Direction: source.Down})
	if errDownVer5 != nil {
		t.Fatal(errDownVer5)
	}
	errMetaDownVer6 := m.Append(&source.Migration{Version: 6, Direction: source.MetaDown})

Review comment (suggestion):
```suggestion
	err := m.Append(&source.Migration{Version: 1, Direction: source.Up})
	if err != nil {
		t.Fatal(errUp)
	}
```

Code after:
package stub

import (
	"testing"

	"github.com/hasura/graphql-engine/cli/migrate/source"
	st "github.com/hasura/graphql-engine/cli/migrate/source/testing"
	"github.com/sirupsen/logrus/hooks/test"
)

func Test(t *testing.T) {
	logger, _ := test.NewNullLogger()
	s := &Stub{}
	d, err := s.Open("", logger)
	if err != nil {
		t.Fatal(err)
	}

	m := source.NewMigrations()
	err = m.Append(&source.Migration{Version: 1, Direction: source.Up})
	if err != nil {
		t.Fatal(err)
	}
	err = m.Append(&source.Migration{Version: 1, Direction: source.Down})
	if err != nil {
		t.Fatal(err)
	}
	err = m.Append(&source.Migration{Version: 1, Direction: source.MetaUp})
	if err != nil {
		t.Fatal(err)
	}
	err = m.Append(&source.Migration{Version: 1, Direction: source.MetaDown})
	if err != nil {
		t.Fatal(err)
	}
	err = m.Append(&source.Migration{Version: 3, Direction: source.Up})
	if err != nil {
		t.Fatal(err)
	}
	err = m.Append(&source.Migration{Version: 4, Direction: source.MetaUp})
	if err != nil {
		t.Fatal(err)
	}
	err = m.Append(&source.Migration{Version: 5, Direction: source.Down})
	if err != nil {
		t.Fatal(err)
	}
	err = m.Append(&source.Migration{Version: 6, Direction: source.MetaDown})
Repository: hasura/graphql-engine
File: cli/migrate/source/stub/stub_test.go (Go)

Code before:

import (
	"testing"

	"github.com/hasura/graphql-engine/cli/migrate/source"
	st "github.com/hasura/graphql-engine/cli/migrate/source/testing"
	"github.com/sirupsen/logrus/hooks/test"
)

func Test(t *testing.T) {
	logger, _ := test.NewNullLogger()
	s := &Stub{}
	d, err := s.Open("", logger)
	if err != nil {
		t.Fatal(err)
	}

	m := source.NewMigrations()
	errUp := m.Append(&source.Migration{Version: 1, Direction: source.Up})
	if errUp != nil {
		t.Fatal(errUp)
	}
	errDown := m.Append(&source.Migration{Version: 1, Direction: source.Down})
	if errDown != nil {
		t.Fatal(errDown)
	}
	errMetaUp := m.Append(&source.Migration{Version: 1, Direction: source.MetaUp})
	if errMetaUp != nil {
		t.Fatal(errMetaUp)
	}
	errMetaDown := m.Append(&source.Migration{Version: 1, Direction: source.MetaDown})
	if errMetaDown != nil {
		t.Fatal(errMetaDown)
	}
	errUpVer3 := m.Append(&source.Migration{Version: 3, Direction: source.Up})
	if errUpVer3 != nil {
		t.Fatal(errUpVer3)
	}
	errMetaUpVer4 := m.Append(&source.Migration{Version: 4, Direction: source.MetaUp})
	if errMetaUpVer4 != nil {
		t.Fatal(errMetaUpVer4)
	}
	errDownVer5 := m.Append(&source.Migration{Version: 5, Direction: source.Down})
	if errDownVer5 != nil {
		t.Fatal(errDownVer5)
	}
	errMetaDownVer6 := m.Append(&source.Migration{Version: 6, Direction: source.MetaDown})
	if errMetaDownVer6 != nil {
		t.Fatal(errMetaDownVer6)
	}
	errUpVer8 := m.Append(&source.Migration{Version: 8, Direction: source.Up})

Review comment (suggestion):
```suggestion
	err = m.Append(&source.Migration{Version: 1, Direction: source.Down})
	if err != nil {
		t.Fatal(errDown)
	}
```

Code after:

import (
	"testing"

	"github.com/hasura/graphql-engine/cli/migrate/source"
	st "github.com/hasura/graphql-engine/cli/migrate/source/testing"
	"github.com/sirupsen/logrus/hooks/test"
)

func Test(t *testing.T) {
	logger, _ := test.NewNullLogger()
	s := &Stub{}
	d, err := s.Open("", logger)
	if err != nil {
		t.Fatal(err)
	}

	m := source.NewMigrations()
	err = m.Append(&source.Migration{Version: 1, Direction: source.Up})
	if err != nil {
		t.Fatal(err)
	}
	err = m.Append(&source.Migration{Version: 1, Direction: source.Down})
	if err != nil {
		t.Fatal(err)
	}
	err = m.Append(&source.Migration{Version: 1, Direction: source.MetaUp})
	if err != nil {
		t.Fatal(err)
	}
	err = m.Append(&source.Migration{Version: 1, Direction: source.MetaDown})
	if err != nil {
		t.Fatal(err)
	}
	err = m.Append(&source.Migration{Version: 3, Direction: source.Up})
	if err != nil {
		t.Fatal(err)
	}
	err = m.Append(&source.Migration{Version: 4, Direction: source.MetaUp})
	if err != nil {
		t.Fatal(err)
	}
	err = m.Append(&source.Migration{Version: 5, Direction: source.Down})
	if err != nil {
		t.Fatal(err)
	}
	err = m.Append(&source.Migration{Version: 6, Direction: source.MetaDown})
	if err != nil {
		t.Fatal(err)
	}
	err = m.Append(&source.Migration{Version: 8, Direction: source.Up})
Repository: hasura/graphql-engine
File: server/src-lib/Hasura/Server/Init/Config.hs (Haskell)

Code before:
class FromEnv a where
  fromEnv :: String -> Either String a

-- Deserialize from seconds, in the usual way
instance FromEnv NominalDiffTime where
  fromEnv s = maybe (Left "could not parse as a Double") (Right . realToFrac) $
                (readMaybe s :: Maybe Double)

instance FromEnv String where
  fromEnv = Right

instance FromEnv HostPreference where
  fromEnv = Right . DataString.fromString

instance FromEnv Text where
  fromEnv = Right . T.pack

instance FromEnv AuthHookType where
  fromEnv = readHookType

instance FromEnv Int where
  fromEnv = maybe (Left "Expecting Int value") Right . readMaybe

instance FromEnv Integer where
  fromEnv = maybe (Left "Expecting Integer value") Right . readMaybe

instance FromEnv AdminSecretHash where
  fromEnv = Right . hashAdminSecret . T.pack

instance FromEnv RoleName where
  fromEnv string = case mkRoleName (T.pack string) of
    Nothing       -> Left "empty string not allowed"
    Just roleName -> Right roleName

instance FromEnv Bool where
  fromEnv = parseStrAsBool

instance FromEnv Q.TxIsolation where
  fromEnv = readIsoLevel

instance FromEnv CorsConfig where
  fromEnv = readCorsDomains

instance FromEnv [API] where
  fromEnv = readAPIs

instance FromEnv LQ.BatchSize where
  fromEnv s = do
    val <- readEither s
    maybe (Left "batch size should be a non negative integer") Right $ LQ.mkBatchSize val


Review comment (suggestion):
We don't need this one anymore :-)
```suggestion
```

Code after:
class FromEnv a where
  fromEnv :: String -> Either String a

-- Deserialize from seconds, in the usual way
instance FromEnv NominalDiffTime where
  fromEnv s = maybe (Left "could not parse as a Double") (Right . realToFrac) $
                (readMaybe s :: Maybe Double)

instance FromEnv String where
  fromEnv = Right

instance FromEnv HostPreference where
  fromEnv = Right . DataString.fromString

instance FromEnv Text where
  fromEnv = Right . T.pack

instance FromEnv AuthHookType where
  fromEnv = readHookType

instance FromEnv Int where
  fromEnv = maybe (Left "Expecting Int value") Right . readMaybe

instance FromEnv AdminSecretHash where
  fromEnv = Right . hashAdminSecret . T.pack

instance FromEnv RoleName where
  fromEnv string = case mkRoleName (T.pack string) of
    Nothing       -> Left "empty string not allowed"
    Just roleName -> Right roleName

instance FromEnv Bool where
  fromEnv = parseStrAsBool

instance FromEnv Q.TxIsolation where
  fromEnv = readIsoLevel

instance FromEnv CorsConfig where
  fromEnv = readCorsDomains

instance FromEnv [API] where
  fromEnv = readAPIs

instance FromEnv LQ.BatchSize where
  fromEnv s = do
    val <- readEither s
    maybe (Left "batch size should be a non negative integer") Right $ LQ.mkBatchSize val

instance FromEnv LQ.RefetchInterval where
  fromEnv x = do
    val <- fmap (milliseconds . fromInteger) . readEither $ x