You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "jsoref (via GitHub)" <gi...@apache.org> on 2023/09/28 23:03:04 UTC

[GitHub] [superset] jsoref commented on a diff in pull request #25452: chore(frontend): Spelling

jsoref commented on code in PR #25452:
URL: https://github.com/apache/superset/pull/25452#discussion_r1340733245


##########
superset-frontend/plugins/plugin-chart-echarts/test/Graph/transformProps.test.ts:
##########
@@ -172,7 +172,7 @@ describe('EchartsGraph transformProps', () => {
     );
   });
 
-  it('should transform chart props for viz with category and falsey normalization', () => {
+  it('should transform chart props for viz with category and falsy normalization', () => {

Review Comment:
   apparently i'm temporarily opinionated. I can't remember why. Typically it's because both spellings are in use.



##########
superset-frontend/src/SqlLab/components/App/index.jsx:
##########
@@ -125,7 +125,7 @@ class App extends React.PureComponent {
 
   componentDidUpdate() {
     const { localStorageUsageInKilobytes, actions, queries } = this.props;
-    const queryCount = queries?.lenghth || 0;
+    const queryCount = queries?.length || 0;

Review Comment:
   scary;
   I think we've talked about this in the past :(



##########
superset-frontend/src/pages/DatasetList/DatasetList.test.tsx:
##########
@@ -197,7 +197,7 @@ describe('DatasetList', () => {
     ).toBeFalsy();
     act(() => {
       wrapper
-        .find('#duplicate-action-tooltop')
+        .find('#duplicate-action-tooltip')

Review Comment:
   notable



##########
superset-frontend/packages/superset-ui-core/test/query/processExtraFormData.test.ts:
##########
@@ -19,7 +19,7 @@
 import { overrideExtraFormData } from '../../src/query/processExtraFormData';
 
 describe('overrideExtraFormData', () => {
-  it('should assign allowed non-existent value', () => {
+  it('should assign allowed nonexistent value', () => {

Review Comment:
   often controversial (I can't find the discussion)



##########
superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx:
##########
@@ -665,7 +665,7 @@ export default function TableChart<D extends DataRecord = DataRecord>(
 
   useLayoutEffect(() => {
     // After initial load the table should resize only when the new sizes
-    // Are not only scrollbar updates, otherwise, the table would twicth
+    // Are not only scrollbar updates, otherwise, the table would twitch

Review Comment:
   I seem to remember discussing this, I'm not sure why it wasn't resolved



##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Heatmap/Heatmap.tsx:
##########
@@ -66,7 +66,7 @@ export const getLayer: getLayerType<unknown> = (
     ?.reverse() as Color[];
 
   return new HeatmapLayer({
-    id: `heatmp-layer-${fd.slice_id}` as const,
+    id: `heatmap-layer-${fd.slice_id}` as const,

Review Comment:
   possibly problematic as it's changing an id



##########
superset-frontend/packages/superset-ui-demo/storybook/stories/plugins/plugin-chart-pivot-table/testData.ts:
##########
@@ -91,7 +91,7 @@ export const basicData = {
   data: [
     {
       location: 'AMEA',
-      program_language: 'Javscript',
+      program_language: 'JavaScript',

Review Comment:
   similarly.
   
   fwiw, I threw out my baseline because I wanted to test the next version of check-spelling...



##########
superset-frontend/src/components/Button/Button.test.tsx:
##########
@@ -50,7 +50,7 @@ describe('Button', () => {
   });
 
   // test stories from the storybook!
-  it('All the sorybook gallery variants mount', () => {
+  it('All the storybook gallery variants mount', () => {

Review Comment:
   brand



##########
superset-frontend/cypress-base/cypress/e2e/sqllab/_skip.sourcePanel.index.test.js:
##########
@@ -23,7 +23,7 @@ describe.skip('SqlLab datasource panel', () => {
     cy.visit('/superset/sqllab');
   });
 
-  // TODO the test bellow is flaky, and has been disabled for the time being
+  // TODO the test below is flaky, and has been disabled for the time being

Review Comment:
   This is actually a word: https://www.merriam-webster.com/dictionary/bellow, but it's almost never right, so I now reject it by default in spell-check-this: https://github.com/check-spelling/spell-check-this/blob/b968ca32e0e5961a85c7fe3e9431f81553c3f434/.github/actions/spelling/reject.txt#L2



##########
superset-frontend/plugins/legacy-plugin-chart-partition/src/Partition.js:
##########
@@ -331,7 +331,7 @@ function Icicle(element, props) {
         tooltip.interrupt().transition().duration(250).style('opacity', 0);
       });
 
-    // When clicking a subdivision, the vis will zoom in to it
+    // When clicking a subdivision, the vis will zoom into it

Review Comment:
   often controversial



##########
superset-frontend/cypress-base/cypress/e2e/dashboard/utils.ts:
##########
@@ -205,9 +205,9 @@ export function expandFilterOnLeftPanel() {
 }
 
 /** ************************************************************************
- * Collapes Native Filter from the left panel on dashboard
+ * Collapses Native Filter from the left panel on dashboard
  * @returns {None}
- * @summary helper for collape native filter
+ * @summary helper for collapse native filter

Review Comment:
   I have a feeling we talked about this in the past, but I'm not certain...



##########
superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx:
##########
@@ -46,7 +46,7 @@ jest.mock('src/dashboard/actions/dashboardState', () => ({
 }));
 jest.mock('src/components/ResizableSidebar/useStoredSidebarWidth');
 
-// mock following dependant components to fix the prop warnings
+// mock following dependent components to fix the prop warnings

Review Comment:
   https://www.merriam-webster.com/grammar/spelling-variants-dependent-vs-dependant
   
   In general, dependant is only right in en-GB and only when referencing a person.



##########
superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx:
##########
@@ -107,7 +107,7 @@ const logAction = () => next => action => {
 
 const createStore = initState =>
   setupStore({
-    disableDegugger: true,
+    disableDebugger: true,

Review Comment:
   scary



##########
superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx:
##########
@@ -356,7 +356,7 @@ test('Should render "annotationsError"', () => {
   render(<SliceHeader {...props} />, { useRedux: true, useRouter: true });
   expect(
     screen.getByRole('img', {
-      name: 'One ore more annotation layers failed loading.',
+      name: 'One or more annotation layers failed loading.',

Review Comment:
   New spell-check-this rule:
   https://github.com/check-spelling/spell-check-this/blob/b968ca32e0e5961a85c7fe3e9431f81553c3f434/.github/actions/spelling/line_forbidden.patterns#L67-L68



##########
superset-frontend/src/dashboard/util/getDashboardUrl.test.js:
##########
@@ -89,15 +89,15 @@ describe('getChartIdsFromLayout', () => {
     windowSpy.mockImplementation(() => ({
       location: {
         origin: 'https://localhost',
-        search: '?unkown_param=value',
+        search: '?unknown_param=value',

Review Comment:
   always unclear when tests do things like this what they're testing...



##########
superset-frontend/src/SqlLab/fixtures.ts:
##########
@@ -29,7 +29,7 @@ import { ISaveableDatasource } from 'src/SqlLab/components/SaveDatasetModal';
 
 export const mockedActions = sinon.stub({ ...actions });
 
-export const alert = { bsStyle: 'danger', msg: 'Ooops', id: 'lksvmcx32' };
+export const alert = { bsStyle: 'danger', msg: 'Oops', id: 'lksvmcx32' };

Review Comment:
   hopefully this doesn't matter



##########
superset-frontend/tools/eslint-plugin-translation-vars/index.js:
##########
@@ -41,7 +41,7 @@ module.exports = {
               context.report({
                 node,
                 message:
-                  "Don't use variables in translation string templates. Flask-babel is a static translation translation service, so it can’t handle strings that include variables",
+                  "Don't use variables in translation string templates. Flask-babel is a static translation service, so it can’t handle strings that include variables",

Review Comment:
   duplicate words are usually typos...



##########
superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx:
##########
@@ -179,7 +179,7 @@ class DatasourceControl extends React.PureComponent {
     const { columns } = datasource;
     // the current granularity_sqla might not be a temporal column anymore
     const timeCol = this.props.form_data?.granularity_sqla;
-    const isGranularitySqalTemporal = columns.find(
+    const isGranularitySqlaTemporal = columns.find(

Review Comment:
   brand



##########
superset-frontend/src/setup/setupApp.ts:
##########
@@ -95,6 +95,6 @@ export default function setupApp() {
   window.jQuery = $;
   require('bootstrap');
 
-  // setup appwide custom error messages
+  // set up app wide custom error messages

Review Comment:
   notable. `setup` is a noun but this wants the verb-phrase, and there's no particularly good reason to merge `app`+`wide`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org