Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

move json files to root #64

Merged
merged 1 commit into from
Aug 28, 2024

Conversation

flashdesignory
Copy link
Collaborator

@flashdesignory flashdesignory commented Aug 22, 2024

Folder organization: this just moves all the json files into a dedicated data folder in the root of the repo.
Nothing else changed.

@felixarntz for awareness and feedback. Hopefully that'll make it easier for the php integration.

@kara

@felixarntz
Copy link
Collaborator

Thanks @flashdesignory! I opened #65 to update the PHP code accordingly. It would be great if you could merge that as soon as you've merged this one, or alternatively feel free to merge it into this PR so we can handle the changes altogether.

Copy link
Collaborator

@housseindjirdeh housseindjirdeh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for doing this 😍

@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import data from './data.json';
import data from '../../../data/google-tag-manager.json';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we consider including absolute imports via alias (e.g. @/data/...)? Might be overkill and something we can worry about in a future PR

import GoogleTagManagerData from './google-tag-manager/data.json';
import GoogleMapsEmbedData from './google-maps-embed/data.json';
import GoogleYoutubeEmbedData from './youtube-embed/data.json';
import GooglaAnalyticsData from '../../data/google-analytics.json';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a typo from a previous PR squeezed through:

Suggested change
import GooglaAnalyticsData from '../../data/google-analytics.json';
import GoogleAnalyticsData from '../../data/google-analytics.json';

@housseindjirdeh housseindjirdeh merged commit e45b33b into GoogleChromeLabs:main Aug 28, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants