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

Create checks to place violations on missing macros in templates #13682

Closed
stoyanK7 opened this issue Sep 1, 2023 · 12 comments · Fixed by #13725
Closed

Create checks to place violations on missing macros in templates #13682

stoyanK7 opened this issue Sep 1, 2023 · 12 comments · Fixed by #13725
Assignees
Milestone

Comments

@stoyanK7
Copy link
Collaborator

stoyanK7 commented Sep 1, 2023

From #13643 (comment)

Can we create a regexp module to place violations on tags in templates, or something similar to catch these?

We need a check to ensure that no examples are shown in templates without being tested. For that, we can place violations on source tags in templates.

@nrmancuso
Copy link
Member

nrmancuso commented Sep 1, 2023

For now, it is probably better to get something in place, fix issues, and have manual suppressions.

@stoyanK7
Copy link
Collaborator Author

stoyanK7 commented Sep 1, 2023

I'd say this check needs to target only source tags in the Examples section. There are numerous places where we have source tags in descriptions and notes.

@nrmancuso
Copy link
Member

nrmancuso commented Sep 1, 2023

I'd say this check needs to target only source tags in the Examples section. There are numerous places where we have source tags in descriptions and notes.

I do not know of some simple way to do this, I think we can do #13682 (comment) and be in good shape for now. We can improve in the future. I am approving this issue simple implementation at #13682 (comment) in mind.

@nrmancuso
Copy link
Member

Removing approved label after some research and consideration.

@nrmancuso nrmancuso removed the approved label Sep 9, 2023
@nrmancuso
Copy link
Member

We currently have no checks on xml template files at all.

  1. Should we create a new checkstyle run/config for these?
  2. What other checks should we have on template files?
  3. Would it be easier/better to check for the presence of <macro name="example">?
  4. Should we create checks for each macro in every template file?
    @rnveach @stoyanK7 @romani share your thoughts here, let's get this in place ASAP

@rnveach
Copy link
Member

rnveach commented Sep 9, 2023

The cache files cache_non_main_files and cachefile (this last one has no templates listed) says those 2 run against all our xdocs. So if we create a new config, we should check that we aren't duplicating what those do already.

Since the xdocs are XML files, I don't see what else we can do besides relying on a bunch of regexps for anything extra which won't be fully reliable.

I am fine with whatever regexps to create.

@romani
Copy link
Member

romani commented Sep 9, 2023

We can have all such validation, but I would rather focus on this at the end, it will be hard to send PR without macroses usage, so easy to spot, so hard to make because as there will be no examples on how to do this :).

@nrmancuso
Copy link
Member

@romani macros will do no good if we do not update templates. We need to have validation in place for existence of macro placement in template where we expect them.

@romani
Copy link
Member

romani commented Sep 9, 2023

I agree, but I would rather focus on migration of all and if any problems happens focus on issues to resolve. And the end catch leftovers by such validation.

@nrmancuso
Copy link
Member

Then we will run into issues in student PRs when we are missing macros in templates, this will not be a good experience for them or maintainers.

@romani romani added the approved label Sep 9, 2023
@romani
Copy link
Member

romani commented Sep 9, 2023

I just say, better to do this after we finish 80-90% migrations. But if somebody can do this earlier it is ok.

@nrmancuso
Copy link
Member

I am on this

@nrmancuso nrmancuso changed the title Create a check to place violations on source tags in templates Create checks to place violations on missing macros in templates Sep 10, 2023
@nrmancuso nrmancuso self-assigned this Sep 10, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Sep 10, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Sep 10, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Sep 10, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Sep 10, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Sep 10, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Sep 10, 2023
@github-actions github-actions bot added this to the 10.12.4 milestone Sep 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants